[PATCH] missing permission checks for net_device.do_ioctl

From: Mitchell Blank Jr (mitch@sfgoth.com)
Date: Tue Mar 07 2000 - 16:24:32 EST


I noticed that the new ethernet bonding device introduced in 49pre1
didn't check for capable(CAP_NET_ADMIN) in its ->do_ioctl method,
letting unprivledged users change the network's configuration.
Since I just completed a similar audit on the atm drivers (which
is merged into the current atm.patch and will be in the kernel in
the next atm merge) I thought I'd take a quick look through
the drivers in drivers/net

net_device.do_ioctl can get called with an argument from SIOCDEVPRIVATE
to SIOCDEVPRIVATE+15 from an unprivledged process. Most of the common
network devices do a reasonable job checking these ioctl's, but there
are exceptions:

First, two possible problems I didn't fix, pending input from maintainers:
  * irda....
    % egrep 'capable|suser' {drivers,include,.}/net/irda/*.[ch] | wc -l
         0
    Need I say more? Dag - could you look at this. At least
    drivers/net/irda/{irport.c,irtty.c,nsc-ircc.c,toshoboe.c,w83977af_ir.c}
    seem to register do_ioctl's that are dangerous. I'm not sure if
    you'd prefer these checks to be placed in the drivers themselves,
    or if its better to put these in the generic code.

  * hamradio/{baycomm_epp.c,hdlcdrv.c}...
    In general the hamradio drivers look pretty good, but both baycomm_epp.c
    and hdlcdrv.c allow setting "calibrate" via the HDLCDRVCTL_CALIBRATE
    sub-ioctl without any permission. It looks to me like that could at
    least be used for possible DoS - Thomas, what's your call?

Things fixed in this patch:

rcpci45.c -- ironically, the driver that seems to have the most problems
        is this VPN driver. The RCU_COMMAND (==SIOCDEVPRIVATE+1) ioctl
        demuxes into several operations including the following that
        probably should be limited to CAP_NET_ADMIN:
          RCUC_SETIPANDMASK, RCUC_SETMAC, RCUC_SETSPEED,
          RCUC_SETPROM, RCUC_SETBROADCAST

slip.c -- sl_ioctl
        SIOCSKEEPALIVE
        SIOCSOUTFILL
        should be protected. SIOCSLEASE probably should be too, but it looks
        like it's safe. Interestingly, slip_ioctl also obviously leaks a
        spinlock if it returns failure, and double-unlocks if SIOCSKEEPALIVE
        suceeds (fixed).

pcmcia/{netwave_cs.c,wavelan_cs.c} --
        The higher-layer wireless extensions prevent unprivledged users
        from setting interface paramaters, but drivers should still
        protect against SIOCGIWENCODE (query encryption key). wavelan.c
        gets it right, but pcmcia/{netwave,wavelan}_cs.c don't.
        
        Also pcmcia/wavelan_cs.c should protect against SIOCSIPROAM.

bonding.c --
        BOND_ENSLAVE, BOND_RELEASE, BOND_SETHWADDR all should be
        protected

dgrs.c --
        DGRS_SETFILTER should be protected

plip.c --
        PLIP_SET_TIMEOUT should be protected

wan/sbni.c -- SIOCDEVRESINSTATS should be protected

I can't say that this patch is tested, but my kernel compiles no worse
with it in.

-Mitch

diff -ur linux-2.3.50pre2-VIRGIN/drivers/net/bonding.c linux-2.3.50pre2mnb1/drivers/net/bonding.c
--- linux-2.3.50pre2-VIRGIN/drivers/net/bonding.c Thu Mar 9 11:22:20 2000
+++ linux-2.3.50pre2mnb1/drivers/net/bonding.c Thu Mar 9 03:47:43 2000
@@ -199,10 +199,16 @@
 
         switch (cmd) {
         case BOND_ENSLAVE:
+ if (!capable(CAP_NET_ADMIN))
+ return -EPERM;
                 return bond_enslave(master, slave);
         case BOND_RELEASE:
+ if (!capable(CAP_NET_ADMIN))
+ return -EPERM;
                 return bond_release(master, slave);
         case BOND_SETHWADDR:
+ if (!capable(CAP_NET_ADMIN))
+ return -EPERM;
                 return bond_sethwaddr(master, slave);
         default:
                 return -EOPNOTSUPP;
diff -ur linux-2.3.50pre2-VIRGIN/drivers/net/dgrs.c linux-2.3.50pre2mnb1/drivers/net/dgrs.c
--- linux-2.3.50pre2-VIRGIN/drivers/net/dgrs.c Thu Feb 10 12:22:03 2000
+++ linux-2.3.50pre2mnb1/drivers/net/dgrs.c Thu Mar 9 03:48:44 2000
@@ -852,6 +852,8 @@
                                 return -EFAULT;
                         return (0);
                 case DGRS_SETFILTER:
+ if (!capable(CAP_NET_ADMIN))
+ return -EPERM;
                         if (ioc.port > privN->bcomm->bc_nports)
                                 return -EINVAL;
                         if (ioc.filter >= NFILTERS)
diff -ur linux-2.3.50pre2-VIRGIN/drivers/net/pcmcia/netwave_cs.c linux-2.3.50pre2mnb1/drivers/net/pcmcia/netwave_cs.c
--- linux-2.3.50pre2-VIRGIN/drivers/net/pcmcia/netwave_cs.c Thu Feb 17 09:18:47 2000
+++ linux-2.3.50pre2mnb1/drivers/net/pcmcia/netwave_cs.c Thu Mar 9 11:29:55 2000
@@ -686,6 +686,10 @@
 #if WIRELESS_EXT > 8 /* Note : The API did change... */
     case SIOCGIWENCODE:
         /* Get scramble key */
+ if (!capable(CAP_NET_ADMIN)) {
+ ret = -EPERM;
+ break;
+ }
         if(wrq->u.encoding.pointer != (caddr_t) 0)
           {
             char key[2];
@@ -725,6 +729,10 @@
 #else /* WIRELESS_EXT > 8 */
     case SIOCGIWENCODE:
         /* Get scramble key */
+ if (!capable(CAP_NET_ADMIN)) {
+ ret = -EPERM;
+ break;
+ }
         wrq->u.encoding.code = scramble_key;
         wrq->u.encoding.method = 1;
         break;
diff -ur linux-2.3.50pre2-VIRGIN/drivers/net/pcmcia/wavelan_cs.c linux-2.3.50pre2mnb1/drivers/net/pcmcia/wavelan_cs.c
--- linux-2.3.50pre2-VIRGIN/drivers/net/pcmcia/wavelan_cs.c Thu Feb 17 09:18:47 2000
+++ linux-2.3.50pre2mnb1/drivers/net/pcmcia/wavelan_cs.c Thu Mar 9 11:40:45 2000
@@ -2068,6 +2068,11 @@
 
     case SIOCGIWENCODE:
       /* Read the encryption key */
+ if(!capable(CAP_NET_ADMIN))
+ {
+ ret = -EPERM;
+ break;
+ }
       if(!mmc_encr(base))
         {
           ret = -EOPNOTSUPP;
@@ -2428,7 +2433,10 @@
 
 #ifdef WAVELAN_ROAMING
     case SIOCSIPROAM:
- /* Note : should check if user == root */
+ if (!capable(CAP_NET_ADMIN)) {
+ ret = -EPERM;
+ break;
+ }
       if(do_roaming && (*wrq->u.name)==0)
         wv_roam_cleanup(dev);
       else if(do_roaming==0 && (*wrq->u.name)!=0)
diff -ur linux-2.3.50pre2-VIRGIN/drivers/net/plip.c linux-2.3.50pre2mnb1/drivers/net/plip.c
--- linux-2.3.50pre2-VIRGIN/drivers/net/plip.c Thu Feb 17 09:18:47 2000
+++ linux-2.3.50pre2mnb1/drivers/net/plip.c Thu Mar 9 03:49:15 2000
@@ -1227,6 +1227,8 @@
                 pc->nibble = nl->nibble;
                 break;
         case PLIP_SET_TIMEOUT:
+ if (!capable(CAP_NET_ADMIN))
+ return -EPERM;
                 nl->trigger = pc->trigger;
                 nl->nibble = pc->nibble;
                 break;
diff -ur linux-2.3.50pre2-VIRGIN/drivers/net/rcpci45.c linux-2.3.50pre2mnb1/drivers/net/rcpci45.c
--- linux-2.3.50pre2-VIRGIN/drivers/net/rcpci45.c Sat Feb 12 15:45:05 2000
+++ linux-2.3.50pre2mnb1/drivers/net/rcpci45.c Thu Mar 9 11:39:28 2000
@@ -1113,6 +1113,8 @@
             break;
         case RCUC_SETIPANDMASK:
             printk("RC SETIPANDMASK\n");
+ if (!capable(CAP_NET_ADMIN))
+ return -EPERM;
             RCUD_SETIPANDMASK = &RCuser.RCUS_SETIPANDMASK;
             printk ("RC New IP Addr = %d.%d.%d.%d, ", (U8) ((RCUD_SETIPANDMASK->IpAddr) & 0xff),
                     (U8) ((RCUD_SETIPANDMASK->IpAddr >> 8) & 0xff),
@@ -1127,6 +1129,8 @@
             break;
         case RCUC_SETMAC:
             printk("RC SETMAC\n");
+ if (!capable(CAP_NET_ADMIN))
+ return -EPERM;
             RCUD_SETMAC = &RCuser.RCUS_SETMAC;
             printk ("RC New MAC addr = %02X:%02X:%02X:%02X:%02X:%02X\n",
                     (U8) (RCUD_SETMAC->mac[0]), (U8) (RCUD_SETMAC->mac[1]), (U8) (RCUD_SETMAC->mac[2]),
@@ -1135,18 +1139,24 @@
             break;
         case RCUC_SETSPEED:
             printk("RC SETSPEED\n");
+ if (!capable(CAP_NET_ADMIN))
+ return -EPERM;
             RCUD_SETSPEED = &RCuser.RCUS_SETSPEED;
             RCSetLinkSpeed(pDpa->id, (U16) RCUD_SETSPEED->LinkSpeedCode);
             printk("RC New speed = 0x%d\n", RCUD_SETSPEED->LinkSpeedCode);
             break;
         case RCUC_SETPROM:
             printk("RC SETPROM\n");
+ if (!capable(CAP_NET_ADMIN))
+ return -EPERM;
             RCUD_SETPROM = &RCuser.RCUS_SETPROM;
             RCSetPromiscuousMode(pDpa->id,(U16)RCUD_SETPROM->PromMode);
             printk("RC New prom mode = 0x%d\n", RCUD_SETPROM->PromMode);
             break;
         case RCUC_SETBROADCAST:
             printk("RC SETBROADCAST\n");
+ if (!capable(CAP_NET_ADMIN))
+ return -EPERM;
             RCUD_SETBROADCAST = &RCuser.RCUS_SETBROADCAST;
             RCSetBroadcastMode(pDpa->id,(U16)RCUD_SETBROADCAST->BroadcastMode);
             printk("RC New broadcast mode = 0x%d\n", RCUD_SETBROADCAST->BroadcastMode);
diff -ur linux-2.3.50pre2-VIRGIN/drivers/net/slip.c linux-2.3.50pre2mnb1/drivers/net/slip.c
--- linux-2.3.50pre2-VIRGIN/drivers/net/slip.c Thu Feb 17 09:18:47 2000
+++ linux-2.3.50pre2mnb1/drivers/net/slip.c Thu Mar 9 11:37:57 2000
@@ -1262,8 +1262,14 @@
         switch(cmd){
         case SIOCSKEEPALIVE:
                 /* max for unchar */
- if (((unsigned int)((unsigned long)rq->ifr_data)) > 255)
+ if (!capable(CAP_NET_ADMIN)) {
+ spin_unlock_bh(&sl->lock);
+ return -EPERM;
+ }
+ if (((unsigned int)((unsigned long)rq->ifr_data)) > 255) {
+ spin_unlock_bh(&sl->lock);
                         return -EINVAL;
+ }
                 sl->keepalive = (unchar) ((unsigned long)rq->ifr_data);
                 if (sl->keepalive != 0) {
                         sl->keepalive_timer.expires=jiffies+sl->keepalive*HZ;
@@ -1272,7 +1278,6 @@
                 } else {
                         del_timer(&sl->keepalive_timer);
                 }
- spin_unlock_bh(&sl->lock);
                 break;
 
         case SIOCGKEEPALIVE:
@@ -1280,9 +1285,15 @@
                 break;
 
         case SIOCSOUTFILL:
- if (((unsigned)((unsigned long)rq->ifr_data)) > 255) /* max for unchar */
+ if (!capable(CAP_NET_ADMIN)) {
+ spin_unlock_bh(&sl->lock);
+ return -EPERM;
+ }
+ if (((unsigned)((unsigned long)rq->ifr_data)) > 255) { /* max for unchar */
+ spin_unlock_bh(&sl->lock);
                         return -EINVAL;
- if ((sl->outfill = (unchar)((unsigned long) rq->ifr_data)) != 0){
+ }
+ if ((sl->outfill = (unchar)((unsigned long) rq->ifr_data)) != 0){
                         mod_timer(&sl->outfill_timer, jiffies+sl->outfill*HZ);
                         set_bit(SLF_OUTWAIT, &sl->flags);
                 } else {
diff -ur linux-2.3.50pre2-VIRGIN/drivers/net/wan/sbni.c linux-2.3.50pre2mnb1/drivers/net/wan/sbni.c
--- linux-2.3.50pre2-VIRGIN/drivers/net/wan/sbni.c Thu Feb 17 09:18:47 2000
+++ linux-2.3.50pre2mnb1/drivers/net/wan/sbni.c Thu Mar 9 11:34:23 2000
@@ -1200,6 +1200,8 @@
                 }
                 case SIOCDEVRESINSTATS:
                 {
+ if (!capable(CAP_NET_ADMIN))
+ return -EPERM;
                         DP( printk("%s: SIOCDEVRESINSTATS\n",dev->name); )
                         lp->in_stats.all_rx_number = 0;
                         lp->in_stats.bad_rx_number = 0;

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Tue Mar 07 2000 - 21:00:23 EST