madison

Insecurity blues: What I learned from my buggy code

Jeremy Allison | November 26, 2007 12:00 AM PST

Summary

Jeremy Allison: There's nothing worse than discovering that your own bad code has caused thousands of people to waste their time rolling out patches. Still, it's easier to find and fix security bugs in open source/free software code than it is in proprietary code.
The opinions expressed here are mine alone, and not those of Google, Inc. my employer.]

Commentary--It hasn't been a good month for my code. Samba, the project I'm responsible for, has had to announce several security flaws. Unfortunately some of them were in code I wrote. I always do a large amount of soul-searching whenever that happens. There's nothing worse than finding out something you were responsible for is the cause of many thousands of people having to waste their time rolling out patches. It always makes me wonder if the time has come to give up this programming lark and end my days peacefully in management, messing up other people's code instead of creating my own.

It's very educational however to look at the causes of the flaws, and try and learn what we can from the bugs and also our reactions to them. Samba is an old program. The initial code was originally written 15 years ago. At that time, modern security problems such as integer wrap (where adding two numbers together can end up with a number smaller than both of them, due to the fixed sizes of integers that processors deal with), or heap overflow vulnerabilities (where overwriting unallocated memory on the program heap can allow a clever attacker to seize control of a program) were unheard of. We knew about buffer overruns (where reading more data into a buffer than was originally allocated for it can cause a security breach) and denial of service attacks, but 1992 was a simpler, less hostile time for network software development. Most initial deployments of Samba were on networks isolated from the main Internet, by technically advanced administrators who knew how to download the source code from the Internet.

The kind of security bugs we get now are much more sophisticated than those that were originally present in the first releases of Samba. We also treat them much more seriously than we used to. In the early days, we (or someone else examining the code) would discover a security problem, and almost immediately we made an announcement of a fixed version. Those were also the days where binary releases of Open Source/Free Software were unknown, as everyone knew how to compile from source code. At least everyone who we were interested in working with did. Our response time for security bugs was usually measured in hours (depending on the timezone of the person reporting the bug). I still remember the first attempt by someone to sabotage Samba with a zero-day exploit posted directly to our mailing list without any warning. Andrew Tridgell (the original author of Samba) was so incensed that he immediately raced to ensure the very next message on the list was the patch for the bug. He succeeded. These days we rarely get zero-day exploits posted openly to lists. The bad guys who discover exploits keep them secret for their own illicit use, and the good guys go through the currently agreed-upon best practice of sending notice to the Samba developers directly (at the mailing address security@samba.org) and work with us in coordinating the announcement of the problem and its fix.

Our response time appears to be much slower these days, even though internally we still usually create an initial fix for the problem within a few hours of seeing the report of the bug. Having Samba Team members in almost all timezones helps with this speed of response. What slows us down these days is our own success. In order to responsibly release a security fix version of Samba we must coordinate with a large number of vendors who are reselling and shipping our code. Most of them have their own patch release schedule and internal security teams who validate the suggested patch and agree on a time to announce the vulnerability to everyone.

Who discovers our security bugs? These days, it's about 50-50 between internal audits by the Samba Team and other Open Source security people and external security researchers, who make their money winning contracts by proving how smart they are by discovering vulnerabilities in Open Source/Free Software projects. The way we respond has changed also. What we do now on seeing a security bug is immediately audit the entire code-base to discover if there are any similar problems, or even similar coding practices that might cause future problems, and re-write or remove all such code. It takes longer, but is much safer in the long run. If you examine parts of the Samba code you'll find common functions that are known to be insecure simply won't compile if added to our code. A set of automated macros warns of any use of known bad functions. Still, we can't depend on automation for protection. Our current security vulnerabilities involve exploiting an assumption in one part of the code to cause a vulnerability in what appears to be a completely separate part of the code, which is actually interdependent. Attackers and security researchers are very clever these days.

Is it easier to find and fix security bugs in Open Source/Free Software code rather than proprietary code? My personal feeling on that is "yes". Being able to examine the intent of the author by looking at the code and comments makes it much easier to see where the intent and the code don't match. That's where the security bugs live. Is Open Source/Free Software code safer than proprietary code? I'd say "yes" to that also. Looking at the number of vulnerabilities is misleading. A good 50 percent of our security bugs are found by our own internal code audits, or someone just looking over some code they're working on and saying to themselves "that looks funny..." Whenever we find something like that, we have to report it to the our vendor community and make a security announcement. It is painful doing this but as all our processes are open we don't have the luxury of silently fixing something and hoping no one will ever notice. Proprietary vendors do have that luxury; they would be insane not to take it, and I'm sure they do. Proprietary code vendors need all the help they can get competing with good Open Source/Free Software.

Samba is still written in C. I've examined the possibility of moving to C++, but to be honest that won't really help for the kind of security bugs that we seem to suffer from. In fact by hiding interdependencies in object classes it may make them more difficult to find. Also I'm not the final decision maker on that and many of the Samba Team hate C++ (Luddites that they are :-). Re-writing in a managed code language like Java or C#.NET would definitely help reduce our security problems but would reduce the performance that people find to be one of the compelling reasons to use Samba. It's no accident that none of the competing CIFS servers are written in managed code either. It would cost us six months to a year to re-write everything and get back to the same level of features, probably with reduced performance. We simply don't have the people or time to do that. For the moment we're stuck with C and we have to make the best of it. If someone is creating a competing Open Source/Free Software CIFS server I'd encourage you to do so in a managed code language as an experiment. If it works as well as I'd hope, we may throw our code away and join you :-).

In order to keep Samba relevant and current with modern practice, we've had to keep re-writing and re-designing core parts of the code. Our upcoming release, Samba 3.2, bears little relation to the original 1992 code and uses modern memory management techniques to avoid heap overflow, buffer overruns and memory leaks, much of which was invented by the Samba Team for internal use. The goal is to keep ourselves secure, as the Samba Team uses Samba for file services ourselves. This is all about scratching our own itch, and helping out everyone else as we do so.

I still feel bad about the mistakes I've made, but I try and redeem myself by using them as examples of how not to write code when I do interviews here at Google. If someone doesn't see the bug it always makes me feel better, but acts as a black mark against them in the interview score. After all, everyone we hire should be able to write code better than me :-). The code is out there to be studied; so learn from my mistakes and don't end up having to coordinate a security announcement. That's one learning experience it's better not to have.


Jeremy Allison is one of the lead developers on the Samba Team, a group of programmers developing an open source Windows compatible file and print server product for UNIX systems. Developed over the Internet in a distributed manner similar to the Linux system, Samba is used by all Linux distributions as well as many thousands of corporations worldwide. Jeremy handles the co-ordination of Samba development efforts and acts as a corporate liaison to companies using the Samba code commercially. He works for Google, Inc. who fund him to work full-time on improving Samba and solving the problems of Windows and Linux interoperability.

Talkback Most Recent of 113 Talkback(s)

  • Great article
    Thank you, Jeremy, for open and instructive account, from which one can learn. Really done in the open source spirit.
    ZDNet Gravatar
    markkerzner
    26th Nov 2007
  • Code snippet
    It would have been interesting to have a read your code snippet with your comments on how the exploit (buffer overrun) succeeded.

    For example, on a parameter pass and within a function call, was strcpy() being called vs strncpy() with len n check that produced the side effect? Or was it void * parameter that was cast incorrectly?

    Thanks Jeremy! happy
    ZDNet Gravatar
    D T Schmitz
    26th Nov 2007
  • Re: Code snippet
    The code snippet would be too large for the article I'm afraid.

    Essentially the problem was reading into a fixed-size buffer which was correctly length checked and sanitized so no possibility of overrun. This buffer was then (via a very covoluted code path) being passed to a function that was outputing a network packet with a shorter fixed buffer size. The incoming data was assumed to be safe - of course it wasn't (we need a C "taint" flag really).

    We don't allow strcpy in Samba, it's a banned function.

    Jeremy.
    ZDNet Gravatar
    JeremyAllison
    26th Nov 2007
  • Could you give us a list of your banned functions? I am sure you are also
    banning:

    strncpy - does not always terminate the string.

    sprintf - just like strcpy, no length check.

    realloc - about any arguments you can throw at it are valid for something.

    Well, would like to have the list of banned functions.

    Do you all the "?" operator?
    ZDNet Gravatar
    DonnieBoy
    26th Nov 2007
  • list
    bcopy, strcpy, strcat, sprintf, strncasecmp are explicitly banned.
    realloc is ok, but we use a wrapper for it that is careful about not losing the original pointer on malloc failure.
    What really helps us is using talloc() for everything. This is a home-grown (tridge created actually) malloc wrapper library that gives C++-list destructors to C code.

    Jeremy.
    ZDNet Gravatar
    JeremyAllison
    26th Nov 2007
  • Thanks, but, I am suprised you do not also ban strncpy(), as it does not
    guarantee termination. Also, another is gets(), as it does not check the length either. Oh, there is also vsprintf().

    There are even some strange ones that give cross platform problems such as strptime().

    And, let us not forget the non thread safe functions like localtime(), which should be replaced by localtime_r().

    I am still puzzling over why you have banned strncasecmp(). I will be trying to figure out why you banned it.

    I will definitely check out talloc()!

    And, finally, sorry I did not reply sooner. I forgot to check back, until I was talking about banned functions.
    ZDNet Gravatar
    DonnieBoy
    29th Nov 2007
  • Rebutting himself
    Mr. Allison observes that these days the "bad guys who discover exploits keep them secret for their own illicit use". But then he congratulates open source code for being safer because "a good 50 percent of our security bugs are found by our own internal code audits". Those found by the bad guys are obviously not being reported.

    So open source code is safer because half of all errors are found by Samba team members, with the other half from people attempting to increase business for their security companies.

    Makes the argument uncompelling, no?!


    Relevant quotes:

    "These days we rarely get zero-day exploits posted openly to lists. The bad guys who discover exploits keep them secret for their own illicit use, and the good guys go through the currently agreed-upon best practice of sending notice to the Samba developers directly (at the mailing address security@samba.org) and work with us in coordinating the announcement of the problem and its fix."

    and:

    "Looking at the number of vulnerabilities is misleading. A good 50 percent of our security bugs are found by our own internal code audits, or someone just looking over some code they're working on and saying to themselves 'that looks funny...'"
    ZDNet Gravatar
    Anton Philidor
    26th Nov 2007
  • Its a silly argument at best.
    Lets not kid ourselves, he is talking about Microsoft when he says proprietary. My bet is that Microsoft has a lot more people looking and working with the code than does Samba. If it comes down to "many eyes", Microsoft has the advantage.

    While I agree that internally half the bugs are found, and I suspect the same is true of Microsoft and their review process.

    In general this is one of the things that puts me off on anything open source. The claims of better security simply is not born out by the facts. Open source code has had as many reported "bugs" as anything else and when guys like Jerremy refuse to accept that fact it tells me they are full of themselves and not being honest.
    ZDNet Gravatar
    No_Ax_to_Grind
    26th Nov 2007
  • Chill out, No_Ax
    Sheesh. There is no mention of the word "Microsoft" in an article, but No_Ax still has to bring it up. What gives, your employer paying you by the word these days ?

    Jeremy.
    ZDNet Gravatar
    JeremyAllison
    26th Nov 2007
  • Great Article And Don't Mind No-Axe
    He's a balanced guy with a chip on both shoulders.

    You could do worse. Wait until you get Loverock Davidson commenting on your articles. Arguing with him is like arguing with a cerebral black hole - if you do it long enough it sucks intelligence out of you. Or better - you know you've made it when Mike Cox, ZDNet's satirist, replies to your articles.

    BTW - the flip side is that Samba is widely used and appreciated - warts and all - in investment banks that I've worked in. And I don't rightly remember all that many warts. In fact, I often wish the rest of the software tools I use were as reliable and secure!

    Regards

    Banjo
    ZDNet Gravatar
    BanjoPaterson
    26th Nov 2007
  • LOOK EVERYONE!!!@#~!~! HE MENTIONS ME!!@#@#!
    Good thing I decided to read zdnet today. Glad to see my fans still mention me happy
    ZDNet Gravatar
    Loverock Davidson
    26th Nov 2007
  • Don't even have to complement Loverock
    Insults appear to please him just as much.
    ZDNet Gravatar
    John L. Ries
    26th Nov 2007
  • He's a bit like Vogon Poetry
    Every time I read his mail I feel my own major intestine, in a desperate attempt to save me, wants to leap straight up through my neck and throttle my brain.

    Ahh, I can already feel my IQ slowly slipping away as I read his response.

    Please make it stop....
    ZDNet Gravatar
    BanjoPaterson
    27th Nov 2007
  • Oh Spank
    Remind me not to be drinking from my Pepsi bottle the next time you chastise Axey there. I laughed so hard it shot out my nose. =-)
    ZDNet Gravatar
    Shelendrea
    26th Nov 2007
  • ZDNet Gravatar
    GuidingLight
    26th Nov 2007

Talkback - Tell Us What You Think

Formatting +
BB Codes - Note: HTML is not supported in forums
  • [b] Bold [/b]
  • [i] Italic [/i]
  • [u] Underline [/u]
  • [s] Strikethrough [/s]
  • [q] "Quote" [/q]
  • [ol][*] 1. Ordered List [/ol]
  • [ul][*] · Unordered List [/ul]
  • [pre] Preformat [/pre]
  • [quote] "Blockquote" [/quote]

The best of ZDNet, delivered

ZDNet Newsletters

Get the best of ZDNet delivered straight to your inbox

Facebook Activity