LWN.net Logo

Coverity catches X Window Security Hole

Coverity catches X Window Security Hole

Posted May 3, 2006 0:48 UTC (Wed) by bk (guest, #25617)
Parent article: 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:

This:

if (getuid() == 0 || geteuid != 0)

Should have been this:

if (getuid() == 0 || geteuid() != 0)

Simple enough for human beings to detect and fix.


(Log in to post comments)

Coverity catches X Window Security Hole

Posted May 3, 2006 0:54 UTC (Wed) by grouch (guest, #27289) [Link]

Soooo, how come Coverity pointed it out instead of you?

Coverity catches X Window Security Hole

Posted May 3, 2006 1:33 UTC (Wed) by bk (guest, #25617) [Link]

So you're admitting that you're unable to differentiate between a function pointer and a function call?

Coverity catches X Window Security Hole

Posted May 3, 2006 2:05 UTC (Wed) by elanthis (guest, #6227) [Link]

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

Posted May 3, 2006 2:33 UTC (Wed) by bk (guest, #25617) [Link]

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.

In that respect the press release by Coverity is a little bit misleading.

Coverity catches X Window Security Hole

Posted May 3, 2006 3:27 UTC (Wed) by drag (subscriber, #31333) [Link]

They found it, nobody else did. They deserve credit for it.

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):
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.

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.

Coverity catches X Window Security Hole

Posted May 3, 2006 3:57 UTC (Wed) by kirkengaard (subscriber, #15022) [Link]

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

Posted May 3, 2006 5:59 UTC (Wed) by TwoTimeGrime (guest, #11688) [Link]

> They found it, nobody else did.

Actually, the OpenBSD people found and fixed it almost two months ago.

See http://www.openbsd.org/cgi-bin/cvsweb/XF4/xc/programs/Xse...

Coverity catches X Window Security Hole

Posted May 3, 2006 6:38 UTC (Wed) by nix (subscriber, #2304) [Link]

... and, as usual, didn't bother to actually tell anyone else.

Coverity catches X Window Security Hole

Posted May 3, 2006 16:37 UTC (Wed) by TwoTimeGrime (guest, #11688) [Link]

> ... and, as usual, didn't bother to actually tell anyone else.

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.

Coverity catches X Window Security Hole

Posted May 3, 2006 6:45 UTC (Wed) by airlied (guest, #9104) [Link]

X fixed it months ago as well, OpenBSD only found about it from X.org...

Coverity catches X Window Security Hole

Posted May 3, 2006 7:39 UTC (Wed) by trey (subscriber, #37500) [Link]

Actually, OpenBSD announced the fix today:

http://marc.theaimsgroup.com/?l=openbsd-security-announce...

Coverity catches X Window Security Hole

Posted May 3, 2006 4:18 UTC (Wed) by dang (subscriber, #310) [Link]

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).

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."

Rapid development -> new bugs all the time

Posted May 3, 2006 4:57 UTC (Wed) by JoeBuck (subscriber, #2330) [Link]

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.

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.

Coverity catches X Window Security Hole

Posted May 3, 2006 14:10 UTC (Wed) by kleptog (subscriber, #1183) [Link]

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.

The other bonus is that if your code is confusing enough to confuse coverity, it probably needs a rewrite anyway :)

Coverity catches X Window Security Hole

Posted May 3, 2006 6:17 UTC (Wed) by job (subscriber, #670) [Link]

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

Posted May 3, 2006 7:32 UTC (Wed) by grouch (guest, #27289) [Link]

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.

Coverity catches X Window Security Hole

Posted May 3, 2006 22:45 UTC (Wed) by dlang (✭ supporter ✭, #313) [Link]

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 do

Copyright © 2012, Eklektix, Inc.
Comments and public postings are copyrighted by their creators.
Linux is a registered trademark of Linus Torvalds