Coverity catches X Window Security Hole
| From: | Sharon Smith <linuxpr-AT-yahoo.com> | |
| To: | linuxpr-AT-yahoo.com | |
| Subject: | News Item: COVERITY CATCHES BIGGEST X WINDOW SECURITY HOLE SINCE 2000 | |
| Date: | Tue, 2 May 2006 10:32:11 -0700 (PDT) |
http://www.coverity.com COVERITY CATCHES BIGGEST X WINDOW SECURITY HOLE SINCE 2000 Research contract from US Department of Homeland Security results in rapid fix to worst case scenario security vulnerability in critical software system SAN FRANCISCO, May 2, 2006 Coverity, Inc., makers of the worlds most advanced and scalable source code analysis solution, today announced that as a result of their contract with US Department of Homeland Security (DHS), the biggest X Window System security vulnerability of the last six years was identified and fixed. Using Coverity Prevent, developers tracked down a critical security vulnerability in the X Window System, a graphical interface used in millions of computers, including most UNIX and Linux systems. The X Window System also ships as an optional GUI with Macintosh computers from Apple. According to Daniel Stone, a release manager for the X.Org Foundation, the vulnerability was one of the most significant vulnerabilities discovered in recent memory, something that we find once every three to six years and is very close to Xs worst case scenarios in terms of security. Coverity exposed vulnerabilities in our code that likely wouldn't have been spotted with human eyes. Its attention to subtle detail throughout the entire codebase even parts you wouldn't normally examine manually makes it a very valuable tool in checking your codebase, and has been of definite benefit to X.Org. The vulnerability was found in versions X11R6.9.0 and X11R7.0.0 during a security analysis of 31 major open source projects that Coverity undertook as part of a DHS initiative. This pair of X Window System versions marked a major milestone when released in December of 2005, as they were the first major updates to the X Window System in more than a decade. After the X.Org development team received the results of the analysis, the vulnerability was fixed within a week. The security hole resulted from a missing parenthesis on a small piece of the program that checked the ID of the user. This flaw, caused by something as seemingly harmless as a missing closing parenthesis, allowed local users to execute code with root privileges, giving them the ability to overwrite system files or initiate denial of service attacks. Coverity Prevent is designed to help computer programmers automatically detect and remove software defects such as security vulnerabilities as the software is being built, said Ben Chelf, CTO of Coverity. Weve implemented a system to analyze the X Window System on a continuous basis to help prevent new defects from entering into the project. In my experience, the X.Org team responded to defects extremely quickly to make their high quality software even better. ## About Coverity Coverity (www.coverity.com), makers of the world's most advanced and scalable source code analysis solution for pinpointing software defects and security vulnerabilities, is a privately-held company headquartered in San Francisco. Coverity was founded in 2002 by leading Stanford University computer scientists whose four-year research project resulted in a breakthrough technique to address the costliest problem in the software industry. That research breakthrough allows developers to quickly and precisely eliminate software defects and security vulnerabilities in tens of millions of lines of new or legacy code. Today, Coverity's solution is used by more than 100 leading companies to significantly improve the quality and security of their software, including Juniper Networks, Symantec/VERITAS, McAfee, Synopsys, NASA, PalmOne, Sun Microsystems and Wind River. Coverity is a registered trademark, and Coverity Extend and Coverity Prevent are trademarks of Coverity, Inc. All other company and product names are the property of their respective owners. Media Contacts Craig Oda Page One PR for Coverity coda@pageonepr.com +1 650 565 9800 x102 Russ Wood Director, Corporate Marketing rwood@coverity.com +1 415 694 5304 --------------------------------- Get amazing travel prices for air and hotel in one click on Yahoo! FareChase
Posted May 2, 2006 21:56 UTC (Tue)
by gorpon (subscriber, #25040)
[Link]
Posted May 2, 2006 22:18 UTC (Tue)
by jwb (guest, #15467)
[Link]
Posted May 2, 2006 22:36 UTC (Tue)
by cortana (subscriber, #24596)
[Link] (5 responses)
Posted May 3, 2006 1:18 UTC (Wed)
by davej (subscriber, #354)
[Link] (2 responses)
neither -Wall or -Wextra will catch this bug. The only thing I've found that prints any warning at all, is splint, which prints the following... gcc is completely silent on the matter.
Posted May 3, 2006 3:46 UTC (Wed)
by jreiser (subscriber, #11027)
[Link]
gcc is completely silent on the matter.
Then that's an `"opportunity for enhancement" in the compiler :-) As long gcc knows that geteuid is not a weak external [such as if there is an "unprotected" call geteuid() anywhere else in the same compilation unit], then there should be a warning that the comparison is between constants and is always true [the address of a non-weak defined subroutine is never 0.]
Posted May 3, 2006 6:24 UTC (Wed)
by Ross (guest, #4065)
[Link]
Posted May 3, 2006 7:37 UTC (Wed)
by trey (guest, #37500)
[Link] (1 responses)
I don't think so. IMHO CVE-2006-1526.
Posted May 3, 2006 16:05 UTC (Wed)
by cortana (subscriber, #24596)
[Link]
Posted May 3, 2006 0:48 UTC (Wed)
by bk (guest, #25617)
[Link] (17 responses)
This:
Should have been this:
Simple enough for human beings to detect and fix.
Posted May 3, 2006 0:54 UTC (Wed)
by grouch (guest, #27289)
[Link] (16 responses)
Posted May 3, 2006 1:33 UTC (Wed)
by bk (guest, #25617)
[Link] (15 responses)
Posted May 3, 2006 2:05 UTC (Wed)
by elanthis (guest, #6227)
[Link] (12 responses)
Posted May 3, 2006 2:33 UTC (Wed)
by bk (guest, #25617)
[Link] (11 responses)
In that respect the press release by Coverity is a little bit misleading.
Posted May 3, 2006 3:27 UTC (Wed)
by drag (guest, #31333)
[Link] (6 responses)
It doesn't matter that it's easily detected by humans when no human has or is going to look closely enough to spot something like that. If they didn't detect it then it would still be sitting there.
This is the whole point behind automated testing.
Remember, get enough eyes and then any code is transparent.. even if some of these eyes are computer applications.
Here is what you can do (I'd try but I don't know enough about this sort of stuff):
If it has happenned once I bet there is a 90% chance that this error has occured in other parts of X.org's code base. Also it probably exists in other applications.
Posted May 3, 2006 3:57 UTC (Wed)
by kirkengaard (guest, #15022)
[Link]
Posted May 3, 2006 5:59 UTC (Wed)
by TwoTimeGrime (guest, #11688)
[Link] (4 responses)
Actually, the OpenBSD people found and fixed it almost two months ago.
See http://www.openbsd.org/cgi-bin/cvsweb/XF4/xc/programs/Xse...
Posted May 3, 2006 6:38 UTC (Wed)
by nix (subscriber, #2304)
[Link] (1 responses)
Posted May 3, 2006 16:37 UTC (Wed)
by TwoTimeGrime (guest, #11688)
[Link]
They might not have known that it could be a security vulnerability. They could have been fixing a typo in the code. As another poster pointed out, X.Org fixed it about two months ago as well. Coverity found it again independently but also recognized the significance of the problem.
Posted May 3, 2006 6:45 UTC (Wed)
by airlied (subscriber, #9104)
[Link]
Posted May 3, 2006 7:39 UTC (Wed)
by trey (guest, #37500)
[Link]
http://marc.theaimsgroup.com/?l=openbsd-security-announce...
Posted May 3, 2006 4:18 UTC (Wed)
by dang (guest, #310)
[Link] (2 responses)
Anyone who brings a tool to the table that can improve bug detection rates has a tool worth praise. And if they bother to run it against tons of open source code and feed back to the developers, then they deserve praise for that as well.
Or put it this way: Coverity just provided X users a very valuable service free of charge. Whether they then market the effort has no bearing on the correct response from X users, which should be "thank you."
Posted May 3, 2006 4:57 UTC (Wed)
by JoeBuck (subscriber, #2330)
[Link]
To get code near-bug-free, you have to stop changing anything about it other than the bugs, and even then you'll introduce some fraction of new bugs per bug fix.
Posted May 3, 2006 14:10 UTC (Wed)
by kleptog (subscriber, #1183)
[Link]
The other bonus is that if your code is confusing enough to confuse coverity, it probably needs a rewrite anyway :)
Posted May 3, 2006 6:17 UTC (Wed)
by job (guest, #670)
[Link]
Posted May 3, 2006 7:32 UTC (Wed)
by grouch (guest, #27289)
[Link] (1 responses)
So you're admitting that you're unable to differentiate between a function pointer and a function call?
So you're not going to answer the question? It's simple, really. You asserted:
You don't really need Coverity to find bugs like this.
[...]
Simple enough for human beings to detect and fix.
Your assertion contradicts facts. The X.Org release manager quoted in the article seems to appreciate Coverity's report. In fact, he is quoted as saying, "Coverity exposed vulnerabilities in our code that likely wouldn't have been spotted with human eyes."
The bug was spotted by Coverity. X.Org was not their customer, yet they did the right thing and reported it to X.Org. An X.Org representative acknowledges the bug existed, is of a type that they "find once every three to six years", and that it was unlikely to be spotted by human eyes poring over the code. Unless you can show where you or anyone else reported it before Coverity, I think I will take the X.Org representative's words at face value over your assertion.
Posted May 3, 2006 22:45 UTC (Wed)
by dlang (guest, #313)
[Link]
Posted May 3, 2006 6:27 UTC (Wed)
by eru (subscriber, #2753)
[Link] (2 responses)
Posted May 3, 2006 6:55 UTC (Wed)
by dberkholz (guest, #23346)
[Link] (1 responses)
http://lists.freedesktop.org/archives/xorg/2006-May/01513...
Posted May 4, 2006 11:32 UTC (Thu)
by daniels (subscriber, #16193)
[Link]
It's in no way Coverity's fault (obviously, they didn't know about this vulnerability until everyone else did).
Posted May 3, 2006 9:14 UTC (Wed)
by arjan (subscriber, #36785)
[Link] (2 responses)
Posted May 3, 2006 12:14 UTC (Wed)
by Wol (subscriber, #4433)
[Link] (1 responses)
Don't forget, the XFree guys were almost "not maintaining" X in the apparent hope it would die. XOrg are now actively maintaining and improving it, so I suspect a lot of your "not so good" code will soon disappear as a result of renewed code churn.
I certainly get the impression that the XFree tree is full of cruft, and the reason for the fork is that the guy running the fork was being actively blocked from cleaning things up.
Cheers,
Posted May 7, 2006 18:19 UTC (Sun)
by bronson (subscriber, #4806)
[Link]
With the modularization of the X tree it beomces easier to rip out, rewrite, and throw away individual pieces. I expect great things will come from x.org in the next few years.
(the quote might be from a Joel on Software article, memory is hazy)
The missing close parenthesis that launched 50,000 emails.Coverity catches X Window Security Hole
I love how this story highlights the great benefit of free software. If you or your company are considering using free software in a critical role, you can hire Coverity to analyze the code before you use it. You would not be able to do so with proprietary software unless you were large enough and wealthy enough to get the vendor to disclose the source code to you.Coverity catches X Window Security Hole
The flaw in question is CVE-2006-0745.
This is good lesson as to why you should always work with the -Wall and -Werror compiler options; I guess Xorg generates so many warnings that the additional 'comparison between pointer and int' went un-noticed. :)
Coverity catches X Window Security Hole
Coverity catches X Window Security Hole
test.c:5:23: Operands of != have incompatible types ([function (void) returns
__uid_t], int): geteuid != 0
Types are incompatible. (Use -type to inhibit warning)
geteuid != 0
Coverity catches X Window Security Hole
Didn't we already have this discussion? This is the same hole that was announced 3-4 weeks back. My opinion is still that GCC should be warning about this usage in -Wall (really anywhere a function is being compared with any constant, unless it's weak and the constant is NULL), and that the X sources should be cleaned up enough that the warning wouldn't be lost in the noise.Deja Vu
"The flaw in question is CVE-2006-0745."Coverity catches X Window Security Hole
That's a separate issue. ;)
Coverity catches X Window Security Hole
You don't really need Coverity to find bugs like this. It's not like there was a missing parentheses in some horrible deeply nested expression, here was the problem:
Coverity catches X Window Security Hole
if (getuid() == 0 || geteuid != 0)
if (getuid() == 0 || geteuid() != 0)
Soooo, how come Coverity pointed it out instead of you?
Coverity catches X Window Security Hole
So you're admitting that you're unable to differentiate between a function pointer and a function call?Coverity catches X Window Security Hole
He's admitting that among 100,000s lines of code, the human eye is likely to just flat out not notice that. If you are looking at the line of code in isolation, the error is easy to spot. Surrounded by many other lines of code when you're not specifically looking for that error, it's something you just are not likely to notice. It's entirely possible to stare directly at the line of code and your subconscious "fills in the blanks" and hides the error from you. That's just how human brains work.Coverity catches X Window Security Hole
I don't necessarily disagree. My point is more that while I'm sure the Coverity tool is useful for finding lots of very difficult and obscure bugs, this doesn't look like one of them. It's as much due to luck that Coverity happened to find this before some human developer as opposed to the inherent superiority of the proprietary Coverity scanner.Coverity catches X Window Security Hole
They found it, nobody else did. They deserve credit for it.Coverity catches X Window Security Hole
Write a program that goes thru the code and looks for this sort of error. A script or perl or whatever.. It just goes through the code looking for errors caused by missing parathesis of this nature.
Hooah. Alan Cox says Open Source software is always late -- here we have a need, and here we have a description for the tool to fix the problem, and so now somebody will hopefully be moved to scratch that particular itch.Coverity catches X Window Security Hole
> They found it, nobody else did.Coverity catches X Window Security Hole
... and, as usual, didn't bother to actually tell anyone else.Coverity catches X Window Security Hole
> ... and, as usual, didn't bother to actually tell anyone else.Coverity catches X Window Security Hole
X fixed it months ago as well, OpenBSD only found about it from X.org...Coverity catches X Window Security Hole
Actually, OpenBSD announced the fix today:Coverity catches X Window Security Hole
The game isn't "find the most obscure bugs" but is rather "find the bugs." If you've got a problem with Coverity, then sift through all of the fixes in linux kernel that come out of Viro's bird tree and see how many of the bugs caught through sparse annotations are obvious to the practiced eye. ( Hint: Viro's comments may help you detect these. ) And think about it. Linux code has more active eyes on it than many open source projects and sparse finds stuff. Is it luck? Would people find this stuff eventually? For some of these, empirically the answer seems to be "no" given how long a few of the bugs have been in the code. And even if someone were to eventually find everything, do you really want to wait? Or do you really think that the tool isn't worthwhile for finding things a hell of a lot sooner? Or spin it this way: Why did Linus invest his time in rolling sparse in the first place? Why does Viro spend time on sparse annotations? Why does mingo invest time writing automated lock checking code? The answer is that for any complex code base, you really need automated code checkers and good regression test suites if you want to find and fix bugs ( and it doesn't matter if the bugs are simple or mind-bendingly complex).Coverity catches X Window Security Hole
Linux (the kernel) is undergoing rapid change, and has large numbers of developers. This is a recipe for introducing new bugs at a rapid rate, both because you can expect a certain number of bugs to be introduced proportional to the number of new lines of code, and because it's a large, complex piece of software with many constraints that developers must keep in mind.
Rapid development -> new bugs all the time
Coverity is a nice tool, it indeed picks up all sorts of logic errors. Unfortunatly, it's not the brightest. Nearly two-thirds of the "issues" it found in PostgreSQL are false positives. But it's this low-hanging stuff thats nice to have an automated tool to check for because no human will see it.Coverity catches X Window Security Hole
I believe the point of static checkers such as Coverity's tool is to catch "low-hanging fruit", i.e. obvious mistakes buried in code and not the really obscure ones.Coverity catches X Window Security Hole
Coverity catches X Window Security Hole
x.org wasn't their customer true, however they were being paied by their customer (department of homeland security) specificly to test the X codebase. so their notification about the bugs was part of what they were paied to doCoverity catches X Window Security Hole
This seems to be the same bug that was discussed over a month ago in this
LWN article:
http://lwn.net/Articles/176233/. It sparked a long discussion
on static checking and the C language. Pointless to repeat it here,
read the old comments...
Recycled news... and recycled discussion.
Because an entirely separate vulnerability was announced this morning.Not just recycled, but deceptive
That's just a phenomenally bad accident of timing. I sent off some wording changes to Coverity a bit over a week ago, but I'd misconfigured my mail server, so all my mail was a week late. By the time I got the mail out, it was the day before we unembargoed the Render fix, so the press release went out that day.Not just recycled, but deceptive
No offence to the X guys, but there is a lot of really not-so-good code in there.. if the coverity guys only found one roothole.. that's surprisingly low ;)They scanned all of X and only found one roothole?
Oh well ... at least the maintainer has now changed ...They scanned all of X and only found one roothole?
Wol
"This is the axe my great grandfather used! Only replaced the head three times and the handle four."They scanned all of X and only found one roothole?