Weekly Edition
Return to the Security page
| |||||||||||
The netfilter packet of death
Adam Osuchowski and Tomasz Dubinski have sent out an advisory regarding a new vulnerability in
the 2.6 netfilter subsystem. Netfilter, being the Linux firewalling code,
inspects network packets and makes decisions on which ones to pass on. Use
of netfilter is supposed to increase security, so it is always discouraging
when the opposite happens. Fortunately, the number of sites vulnerable to
this particular bug should be fairly small.
TCP packets can contain an "options" field within the header. This field allows TCP implementations to change how the protocol works; options can be used to turn on features like selective acknowledgments, change how checksumming is done, and so on. Each option has a simple format:
Multiple options can be packed into the field; an option number of zero terminates the list. If netfilter is asked to filter packets based on the contents of the TCP options field, it goes into a loop stepping through each option present in a packet. Unfortunately, it treats the length byte as a signed quantity; the result is that, with an option number greater than 128, netfilter's index into the options field can be pushed backward, and the code can end up in an infinite loop. That tends to slow packet delivery somewhat. The fix is straightforward: declare the options array as unsigned. The good news is that, in all likelihood, very few firewalls filter on the TCP options field, and, of those, most have probably not yet been upgraded to 2.6. The bad news is that there are almost certainly many other bugs in the kernel (and elsewhere) caused by confusion between signed and unsigned types. These vulnerabilities can be hard to find without detailed, tedious auditing. And some of them, certainly, will have a larger impact than this one. (Log in to post comments)
The netfilter packet of death Posted Jul 1, 2004 6:06 UTC (Thu) by eru (subscriber, #2753) [Link] According to the description, this bug was the result of correct declaration being turned into an incorrect one at some point in the kernel evolution:There is only need to change type of `opt' array from signed char to unsigned (or, better to u_int8_t) as it was defined in 2.4 kernel or prior to version 1.16 of net/ipv4/netfilter/ip_tables.c file.I find the fact that this kind of regression can happen more worrying than the bug itself. Part of the blame goes to the C language definition that makes it less convenient to declare unsigned bytes than ambiguously signed bytes. I have seen dozens of bugs caused by this. If I could magically change just one thing in the C standard and all implementations, it would be making char be always unsigned.
The netfilter packet of death Posted Jul 1, 2004 6:25 UTC (Thu) by donwaugaman (subscriber, #4214) [Link] With gcc, you can use the -funsigned-char flag to treat any 'char' variables as unsigned.
The netfilter packet of death Posted Jul 1, 2004 9:12 UTC (Thu) by eru (subscriber, #2753) [Link] Yes, and many other C compilers also have an equivalent option. But such options are not useful for anything except compiling old code that was written in nonportable fashion for some C implementation that made chars unsigned. The C standard unfortunately allows char to be either signed or unsigned, and does not require implementations to have any option or pragma for specifying this behaviour. So you cannot rely on knowing the signedness property of char, if you want to write portable code.
The netfilter packet of death Posted Jul 2, 2004 3:35 UTC (Fri) by khim (subscriber, #9252) [Link] But linux kernel was never portable across C compilers! It uses a lot of GCC extensions anyway...
The netfilter packet of death Posted Jul 1, 2004 13:10 UTC (Thu) by clugstj (subscriber, #4020) [Link] Even if "char" was unsigned by default, a non-obvious bug can be createdby assuming it is signed. A signed-by-default char is completely consistent with the other integral types (short is signed, int is signed, long is signed). The problem was that someone changed the type from "u_int8_t" to "char". Unless you make comparisons between signed and unsigned types illegal, the compiler can't find this type of bug.
The netfilter packet of death Posted Jul 2, 2004 1:03 UTC (Fri) by giraffedata (subscriber, #1954) [Link] Unless you make comparisons between signed and unsigned types illegal, the compiler can't find this type of bug.The bug in question here isn't that one. You're telling the compiler that a certain 8 bits are the binary representation of a signed integer (with 8 bit precision). It's a lie, though, because according to the TCP standard, the 8 bits stand for an unsigned number. So when you add the "11111111" to the cursor, you should be adding 255 but instead you're subtracting 1. Making either arithmetic or comparison between signed and unsigned illegal is about as sensible a way to solve this problem as eliminating one or the other type altogether. Comparing or adding an unsigned and a negative number are legitimate things for a program to do. On the other hand, making the compiler treat a signed integer as a signed integer when comparing it to an unsigned one (even if it requires additional instructions) would be a reasonable fix for that problem.
The netfilter packet of death Posted Jul 2, 2004 7:46 UTC (Fri) by eru (subscriber, #2753) [Link] Even if "char" was unsigned by default, a non-obvious bug can be created by assuming it is signed. A signed-by-default char is completely consistent with the other integral types (short is signed, int is signed, long is signed). But char is special in that its most common use is to store characters, and talking about the sign of characters makes about as much sense as talking about the colour of floating-point numbers. I think the confusion possibility you mention would be pretty unlikely for this reason. C got away with this madness for years only because it was most commonly used with a 7-bit character set on implementations with 8-bit char type, so the integer values corresponding to characters are positive no matter how char's sign bit is interpreted. But 7-bit only character sets have long been obsolete, at least outside of U.S. (I first became aware of char's strange properties about 20 years ago when I wondered why some text-processing programs I had compiled for a PC behaved badly with my Finnish texts, without any obvious flaw I could see then).
Never index with signed type Posted Jul 1, 2004 19:41 UTC (Thu) by NAR (subscriber, #1313) [Link] The bad news is that there are almost certainly many other bugs in the kernel (and elsewhere) caused by confusion between signed and unsigned types.The C compiler really should produce at least a warning when an array is indexed with a variable of a signed type...
Never index with signed type Posted Jul 2, 2004 15:49 UTC (Fri) by shane (subscriber, #3335) [Link] That would produce an almost infinite number of warnings in lots of programs. Using int is pretty common in loops - I do it myself.
Never index with signed type Posted Jul 5, 2004 9:17 UTC (Mon) by NAR (subscriber, #1313) [Link] Well, maybe I used to much C++, but there you get a warning for code like this:
for(int i=0; i<array.size(); ++i) {
...
}
because the size() method returns size_t, so nowadays I use size_t in for loops...
Anyway, this warning could be turned off if the code uses so much int indexes.
|
|||||||||||