lkml.org 
[lkml]   [2000]   [Mar]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
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/

\
 
 \ /
  Last update: 2005-03-22 13:56    [W:0.039 / U:1.240 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site