Slashdot Mirror
Homeland Security Uncovers Critical Flaw in X11
Amy's Robot writes "An open-source security audit program funded by the U.S. Department of
Homeland Security has flagged a critical vulnerability in the X Window System (X11) which is used in Unix and Linux systems. A missing parentheses in a bit of code is to blame. The error can grant a user root access, and was discovered using an automated code-scanning tool." While serious, the flaw has already been corrected.
Check the CVS server. OpenBSD 0wns again!
it was a missing pair of parentheses:
h es/xorg-server-1.0.1-geteuid.diff
a rch/013992.html
http://xorg.freedesktop.org/releases/X11R7.0/patc
http://lists.freedesktop.org/archives/xorg/2006-M
if you said a + b * c but you really wanted (a + b) * c the compiler won't bleat.
Engineering is the art of compromise.
You're misinterpreting what the problem was. It was a change from this:
if (getuid() == 0 || geteuid != 0)
to this:
if (getuid() == 0 || geteuid() != 0)
The fix was posted before, but the problem was that someone used "geteuid" rather than "geteuid()".
This results in making use of the function address rather than the return value of the function, which could cause difficulties.
What a novel concept
g 2006-03-17 23:30:10.0000 /* First the options that are only allowed for root */
actual code patch:
--- programs/Xserver/hw/xfree86/common/xf86Init.c.ori
00000 +0200
+++ programs/Xserver/hw/xfree86/common/xf86Init.c 2006-03-17 23:29:35.0000
00000 +0200
@@ -1376,7 +1376,7 @@
}
- if (getuid() == 0 || geteuid != 0)
+ if (getuid() == 0 || geteuid() != 0)
{
if (!strcmp(argv[i], "-modulepath"))
{
@@ -1679,7 +1679,7 @@
}
if (!strcmp(argv[i], "-configure"))
{
- if (getuid() != 0 && geteuid == 0) {
+ if (getuid() != 0 && geteuid() == 0) {
ErrorF("The '-configure' option can only be used by root.\n");
exit(1);
}
Bug:
https://bugs.freedesktop.org/show_bug.cgi?id=6213
If you're running Gentoo stable, then you're safe: you've got Xorg 6.8.2, which is not vulnerable.
If you're running ~x86, then you've got the vulnerable version. It's a local exploit, one that is trivially simple for an experienced programmer to use.
"They redundantly repeated themselves over and over again incessantly without end ad infinitum" -- ibid.
OSX ships XFree86 4.3.0, which is not vulnerable.
"They redundantly repeated themselves over and over again incessantly without end ad infinitum" -- ibid.
All of the affected versions are masked for testing under Gentoo, so chances are that you're not using an affected version anyway. In any case, it's evidently trivial for a local user starting an xserver to cause it to execute arbitrary code, but there's no way to attack a running server locally or remotely.
Doh! I missed the euid. please mod the above post to oblivion
Check again, getuid() and geteuid() are not the same, so:
if (getuid() == 0 || geteuid() != 0)
means something like if the real user id executing the process is 0 (root), or if the effective user id of the process is not 0 (root), then execute the following code.
See here and here.
I'm not quite sure what the difference is between the real and the effective user id, perhaps someone can enlighten us.
I love my sig.
The effective UID (euid) is changed when you run a setuid app, while the real UID (uid in this case, or ruid) is not.
The effective UID is normally associated with permission to access files. Well, Linux actually uses the filesystem UID (fsuid or fuid) for that, but that one nearly always tracks the effective UID for compatibility.
There is also a saved UID (suid or svuid) that is helpful for apps that need to swap UIDs back and forth. It's not used for anything else.
Incidentally, that's the word you should have used too.
You said:
That's exactly what he said! He said he's sick of code spitting out hundreds of warnings when you compile. In other words, FIX THE CODE so that it doesn't spit out warnings. The point here is that not only did the developers of this code allow it to compile with warnings, but they didn't even check what those warnings were to verify that they were benign! And let's face it, if you take the time to check on a warning, it doesn't take too much extra time to make it so there is no longer a warning.
> (And yes, gcc will throw a warning if you compare a function pointer with 0 instead of NULL)
What version of gcc are you running? Did you actually try it or did you just assume that it would be true?
If you compare a pointer against an integer constant you'll get a warning. However, "0" is special in ANSI C since it's a synonym for NULL. It's perfectly valid to compare a pointer of any type against zero. Some static-analsis tools like Linus's "sparse" are more picky but I don't believe any versions of gcc would ever emit a warning about that. Try it:
extern int foo(const char *a);
int xxx(void) { return foo == 0; }
The exploit mentioned in this article cannot be exploited by a user who isn't logged into your system - you have to be able to run the Xorg command with certain options. See X.Org's advisory at http://lists.freedesktop.org/archives/xorg/2006-Ma rch/013992.html
Rumor has it the ISO C++ committee is likely to pass through a proposal for a new keyword, nullptr, which will have a "magic" type "pointer-to-anything" and has the value of the null pointer constant.
// #1 // #2
// # calls #2 // calls #1
So, E.g.:
struct A;
int f( A* );
int f( int );
int m = f( 0 );
int n = f( nullptr );
Of course, that wouldn't help in the aforementioned case. 0 will still be convertible to a pointer type as it is now; it's just that 'nullptr', being a pointer itself, makes for a "better" conversion to a real pointer type.
nullptr is supposed to be a non-disruptive pure extension (except for the fact that it breaks code that uses 'nullptr' as an identifier) -- meaning that it should not change the meaning of existing code.
A linux terminal server need only the X libraries, not even a single instance of an X server, which generally requires elevated privileges to run. I think I've seen work to correct that, but as it stands at large an X server runs as root and has to arbitrate security, whereas X applications linked to X libraries, displaying to a thin client over the network, the server has no root level code and only the thin client filesystem/system is at any risk.
XML is like violence. If it doesn't solve the problem, use more.
Yes ANSI C requires NULL to be 0, but isn't it usually typed as a void pointer? If I remember right, void pointers normally aren't compatible with function pointers (unless gcc has "fixed" that deficiency).
/usr/include/linux/stddef.h on my Ubuntu box:
Here are the relevant lines from
#undef NULL
#if defined(__cplusplus)
#define NULL 0
#else
#define NULL ((void *)0)
#endif
You missed the point. The value of NULL is 0, but what is a NULL reference?
Conventional C programmers (not C++) define NULL as (void *) 0x0.
http://lists.freedesktop.org/archives/xorg/2006-Ap ril/014874.html
should be applied next, otherwise the bug is still there
I don't know about ANSI, but ISO/IEC 9899:1999(E) (a.k.a. "C99"), under section 7.17 "Common definitions <stddef.h>" states:
Under section 6.3.2.3 "Pointers", the "null pointer constant" is defined as follows:http://outcampaign.org/
Actually, global symbols are known to be non-NULL.
This is defined in the C standard so the compiler can optimize the test away.
This is quite usefull for writting efficient macros (or inlined functions) in
which an argument can be either a function pointer or a global function:
#define call_if_exist(fun,arg) if (fun!=0) { fun(arg) ; }
Then the compiler is not compliant with the standard. Since it defined the constant 0 (and only the constant 0 not for example 1-1) in a pointer context as being converted to the NULL pointer at compile time. The only times 0 isn't correct is as an argument to a function with no prototype (which no one does anymore, right :) and as an argument to a varargs function call - since in both those cases there is no pointer context to trigger the conversion.
You need a better compiler.