Messages in this thread Patch in this message | | | Date | Tue, 7 Mar 2000 13:24:32 -0800 | From | Mitchell Blank Jr <> | Subject | [PATCH] missing permission checks for net_device.do_ioctl |
| |
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/
| |