On Tue, Mar 07, 2000 at 01:24:32PM -0800, Mitchell Blank Jr wrote:
> 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
Good move, I rewally do appreciate...
> 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.
Yes, netwave_cs got it wrong. On the other hand, wavelan_cs
got it right, as far as 2.3.49. I'll show in a minute. Dag won't fix
the Netwave, and I may decide to look into it, but can't promise
anything. I'll also send a patch to Alan block the Get-Encode before
the driver, to avoid this kind of nasty.
> Also pcmcia/wavelan_cs.c should protect against SIOCSIPROAM.
Yep. I don't use this feature, but mea culpa. Send the patch
away, I'll update the Pcmcia package.
> 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;
Perfect, nothing to say...
> 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;
That's wrong. The current code look like :
-------------------------------------------
case SIOCGIWENCODE:
/* Read the encryption key */
if(!mmc_encr(base))
{
ret = -EOPNOTSUPP;
break;
}
/* only super-user can see encryption key */
if(!capable(CAP_NET_ADMIN))
{
ret = -EPERM;
break;
}
-------------------------------------------
This allow to know if the card support encryption or not even
as a normal user, which I don't consider a sensitive information.
> @@ -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)
Perfect...
Thanks for the good job !
Jean
-
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