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

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

[Hank Leininger]

On 2001-06-03, Dawson Engler <engler@csl.Stanford.EDU> 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 <hlein@progressive-comp.com> 
  

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 <engler@csl.Stanford.EDU> 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.c	2001/06/09 18:05:08	1.1
+++ random.c	2001/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 int	len;
 
 	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;

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

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

[Florian Weimer]

Alan Cox <alan@lxorguk.ukuu.org.uk> 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 	                  Florian.Weimer@RUS.Uni-Stuttgart.DE
University of Stuttgart           http://cert.uni-stuttgart.de/
RUS-CERT                          +49-711-685-5973/fax +49-711-685-5898

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

[Alan Cox]

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

[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;