Re: [CHECKER] security rules? (and 2.4.5-ac4 security bug)

[Dawson Engler]

> Indeed; the bug in the uuid_strategy which you pointed out in the
> random driver wasn't caused by the fact that we were using a
> user-specified length (since the length was being capped to a maximum
> value of 16).  The security bug was that the test was done on a signed
> value, and copy_to_user() takes an unsigned value.
> 
> So your checker found a real bug, but it wasn't the one that the
> checker thought it was.  :-)

No, it was the bug the checker thought it was: a signed integer from
user space that had only been upper-bound checked.  If the value had
been unsigned, or had been checked in a range lower_bound < x <
upper_bound there woulnd't have been a message.

But I certainly concede that the message could be more informative.

Dawson
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [CHECKER] security rules? (and 2.4.5-ac4 security bug)

[Theodore Tso]

On Mon, Jun 04, 2001 at 08:20:01AM -0400, Hank Leininger wrote:
> On 2001-06-03, Dawson Engler <[EMAIL PROTECTED]> wrote:
> 
> > Additionally, do people have suggestions for good security rules?
> > We're looking to expand our security checkers.  Right now we just have
> > checkers that warn when:
> 
> Do you already have checks for signed/unsigned issues?  Those often result
> in security problems, although you may already be checking for them simply
> for reliable-code purposes.  ...Hm, looking at the archives, I see Chris
> Evans responded about signedness issues when you asked last month :-P

Indeed; the bug in the uuid_strategy which you pointed out in the
random driver wasn't caused by the fact that we were using a
user-specified length (since the length was being capped to a maximum
value of 16).  The security bug was that the test was done on a signed
value, and copy_to_user() takes an unsigned value.

So your checker found a real bug, but it wasn't the one that the
checker thought it was.  :-)

Alan, I assume you've fixed this already, but here's a patch in case
you haven't.  Note this also fixes the problem the problem pointed out
by Florian Weimer about copy_to_user being passed a null pointer in
the RANDOM_UUID case.

- Ted

--- random.c2001/06/09 18:05:08 1.1
+++ random.c2001/06/09 18:05:19
@@ -1793,7 +1793,7 @@
 void *newval, size_t newlen, void **context)
 {
unsigned char   tmp_uuid[16], *uuid;
-   int len;
+   unsigned intlen;
 
if (!oldval || !oldlenp)
return 1;
@@ -1810,7 +1810,7 @@
if (len) {
if (len > 16)
len = 16;
-   if (copy_to_user(oldval, table->data, len))
+   if (copy_to_user(oldval, uuid, len))
return -EFAULT;
if (put_user(len, oldlenp))
return -EFAULT;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [CHECKER] security rules? (and 2.4.5-ac4 security bug)

[Florian Weimer]

Alan Cox <[EMAIL PROTECTED]> writes:

> n /u2/engler/mc/oses/linux/2.4.5-ac4/drivers/char/random.c:1813:uuid_strategy: 
>ERROR:RANGE:1809:1813: Using user length "len" as argument to "copy_to_user" 
>[type=LOCAL] set by 'get_user':1813
> 
> Sigh I thought I had all of the sysctl ones

BTW uuid_strategy() is broken in the RANDOM_UUID case.  It calls
copy_to_user() on table->data, which is always NULL.

-- 
Florian Weimer[EMAIL PROTECTED]
University of Stuttgart   http://cert.uni-stuttgart.de/
RUS-CERT  +49-711-685-5973/fax +49-711-685-5898
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [CHECKER] security rules? (and 2.4.5-ac4 security bug)

[Hank Leininger]

On 2001-06-03, Dawson Engler <[EMAIL PROTECTED]> wrote:

> Additionally, do people have suggestions for good security rules?
> We're looking to expand our security checkers.  Right now we just have
> checkers that warn when:

Do you already have checks for signed/unsigned issues?  Those often result
in security problems, although you may already be checking for them simply
for reliable-code purposes.  ...Hm, looking at the archives, I see Chris
Evans responded about signedness issues when you asked last month :-P

You may want to check out and/or subscribe to the security-audit list; most
of the discussion is about userland security issues but kernel problems (or
potential  ones) are discussed as well.  We have archives of the list at:
http://marc.theaimsgroup.com/?l=linux-security-audit&r=1&w=2
And see http://www.linuxhelp.org/lsap.shtml for more info, subscribing,
etc.

--
Hank Leininger <[EMAIL PROTECTED]> 
  
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[CHECKER] security rules? (and 2.4.5-ac4 security bug)

[Dawson Engler]

Hi All,

Enclosed is a potential security hole in 2.4.5-ac where an integer from
user space is used as a length argument to copy_to_user.

Additionally, do people have suggestions for good security rules?
We're looking to expand our security checkers.  Right now we just have
checkers that warn when:

1. user pointers are dereferenced

2. an integer from user space is used as a length argument to
   copy*user or as an array index. (this is getting extended
   to include data from network packets)

3. user input can trigger a known bug (e.g., the failed release of
a lock, or a copy_*_user call with interrupts disabled).

more preliminary:
(4) a checker that derives when you're supposed to
do an capable? call and warns when you don't.

(5) checkers to find typical format string bugs.

I'm sure there are a huge set of security holes that are not covered by
these sorts of checks, so if anyone has suggestions, please let us know.

Dawson

PS Someone from world.std.com (I believe) sent a nice rule yesterday,
   but I accidently deleted the message --- could you please resend?


[BUG]
/u2/engler/mc/oses/linux/2.4.5-ac4/drivers/char/random.c:1813:uuid_strategy: 
ERROR:RANGE:1809:1813: Using user length "len" as argument to "copy_to_user" 
[type=LOCAL] set by 'get_user':1813

uuid[8] = 0;
}
if (uuid[8] == 0)
generate_random_uuid(uuid);

Start --->
get_user(len, oldlenp);
if (len) {
if (len > 16)
len = 16;
Error --->
if (copy_to_user(oldval, table->data, len))
return -EFAULT;
if (put_user(len, oldlenp))
return -EFAULT;

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [CHECKER] security rules?

[Paul Mackerras]

William Ie writes:

> 4.linux/2.4.3/drivers/net/ppp_async.c:345:ppp_async_ioctl
> case PPPIOCGFLAGS:
>   val = ap->flags | ap->rbits;
>   if (put_user(val, (int *) arg))
>   break;
>   err = 0;
>   break;
> case PPPIOCSFLAGS:
>   if (get_user(val, (int *) arg))
>   break;
>   ap->flags = val & ~SC_RCV_BITS;
>   spin_lock_bh(&ap->recv_lock);
>   ap->rbits = val & SC_RCV_BITS;
>   spin_unlock_bh(&ap->recv_lock);
>   err = 0;
>   break;
> seems to be getting and setting some flags without CAP_NET_ADMIN like in
> ppp_synctty.c

It is OK because this is a channel ioctl routine called from
ppp_generic.c as a result of an ioctl call on /dev/ppp, and it is not
possible to open /dev/ppp unless you have CAP_NET_ADMIN.

Paul.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[CHECKER] security rules?

[William Ie]

Hi, my name is William Ie and I am currently working with the mc group. We
are currently looking into how capability checks are used in the 2.4.3
kernel. Along the way, we found a few potential bugs that we are not too
sure about. Please bear in mind our checker right now is still very very
crude. 
1.
linux/2.4.3/drivers/block/cciss.c:381:cciss_ioctl:
put_user(hba[ctlr]->hd[MINOR(inode->i_rdev)].start_sect,
&geo->start);
return 0;
case BLKGETSIZE:
if (!arg) return -EINVAL;
put_user(hba[ctlr]->hd[MINOR(inode->i_rdev)].nr_sects,
(long*)arg);
return 0;
case BLKRRPART:
return revalidate_logvol(inode->i_rdev, 1);
Error --->
case BLKFLSBUF:
our analysis indicates that servicing of the BLKRRPART requires the
CAP_SYS_ADMIN capability check, which is not being done here. Is this a
bug?
2.
linux/2.4.3/drivers/block/cpqarray.c:1182:ida_ioctl:

case IDAGETDRVINFO:
return
copy_to_user(&io->c.drv,&hba[ctlr]->drv[dsk],sizeof(drv_info_t));
case BLKGETSIZE:
if (!arg) return -EINVAL;

put_user(ida[(ctlri_rdev, 1);
Error --->
case IDAPASSTHRU:
if (!suser()) return -EPERM;
error = copy_from_user(&my_io, io, sizeof(my_io));
if (error) return error;
error = ida_ctlr_ioctl(ctlr, dsk, &my_io);

Same as above.
3.
linux/2.4.3/net/rose/af_rose.c:1290:rose_ioctl:
case SIOCRSSCAUSE: {
struct rose_cause_struct rose_cause;
if (copy_from_user(&rose_cause, (void *)arg,
sizeof(struct rose_cause_struct)))
return -EFAULT;
sk->protinfo.rose->cause  = rose_cause.cause;
sk->protinfo.rose->diagnostic =
rose_cause.diagnostic;
return 0;
}
The code appears to us to be configuring the device without CAP_NET_ADMIN
capability although we are not too sure that this is actually ok.
4.linux/2.4.3/drivers/net/ppp_async.c:345:ppp_async_ioctl
case PPPIOCGFLAGS:
val = ap->flags | ap->rbits;
if (put_user(val, (int *) arg))
break;
err = 0;
break;
case PPPIOCSFLAGS:
if (get_user(val, (int *) arg))
break;
ap->flags = val & ~SC_RCV_BITS;
spin_lock_bh(&ap->recv_lock);
ap->rbits = val & SC_RCV_BITS;
spin_unlock_bh(&ap->recv_lock);
err = 0;
break;
seems to be getting and setting some flags without CAP_NET_ADMIN like in
ppp_synctty.c
5.
linux/2.4.3/drivers/isdn/isdn_ppp.c:478:isdn_ppp_ioctl
case PPPIOCGFLAGS:  /* get configuration flags */
if ((r = set_arg((void *) arg,
&is->pppcfg,sizeof(is->pppcfg) )))
return r;
break;
Error --->
case PPPIOCSFLAGS:  /* set configuration flags */
if ((r = get_arg((void *) arg, &val,
sizeof(val) ))) {
return r;
}
if (val & SC_ENABLE_IP && !(is->pppcfg &
SC_ENABLE_IP) && (is->state & IPPP_CONNECT)) {
Same as above.
6.linux/2.4.3/drivers/net/ppp_generic.c:550:ppp_ioctl


case PPPIOCGFLAGS:
val = ppp->flags | ppp->xstate | ppp->rstate;
if (put_user(val, (int *) arg))
break;
err = 0;
break;

Error --->
Same as above.



-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [CHECKER] security rules?

[Chris Evans]

Hi Dawson,

Excellent project.

Can I suggest that you check for signedness issues? A typical signature of
a signedness problem is:

int i = get_from_userspace_somehow();
/* Sanity check i */
if (i > MAX_LEN_FOR_I)
  goto bad_bad_out;
/* Bug here!! i can be negative! */


I suspect you find a lot of these sort of errors. I've already nailed a
few.

Cheers
Chris

On Fri, 13 Apr 2001, Dawson Engler wrote:

>
> We're looking at making a set of security checkers.  Does anyone have
> suggestions for good things to go after in addition to the usual
> copy_*_user and buffer overrun bugs?  For example, are there any
> documents that describe the rules for when/how 'capable' is supposed to
> be used?
>
> Thanks for any help,
> Dawson
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [EMAIL PROTECTED]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[CHECKER] security rules?

[Dawson Engler]

We're looking at making a set of security checkers.  Does anyone have
suggestions for good things to go after in addition to the usual
copy_*_user and buffer overrun bugs?  For example, are there any
documents that describe the rules for when/how 'capable' is supposed to
be used?

Thanks for any help,
Dawson
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/