Skip to contentSkip to conversation

security rules? (and 2.4.5-ac4 security bug)

CHECKER Hank Leininger 3 participants 3 messages Jun 4, 2001 – Jun 10, 2001

lore
Share & tools
lore
#0 lore raw
Quoted 3 lines · Dawson Engler

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]>

#1 lore raw
Quoted 10 lines · Hank Leininger
Quoted 3 lines · Dawson Engler

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 +2 -2
@@ -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;
#2 lore raw
Quoted 8 lines

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