security rules? (and 2.4.5-ac4 security bug)
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]>
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 ;
Quoted 8 lines
Indeed; the bug in the
uuid_strategywhich 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, andcopy_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