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

[Theodore Tso <tytso@mit.edu>]

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;