"Steam will save the world"

I was reminded last night, that there are always going to be some constructs that your static analysis tools won’t save you from. [A point made by Microsoft's Michael Howard, in his blog and in his new book on the Secure Development LifeStyle... er... LightCycle... er... LifeCycle]


For instance, here’s a piece of code:

#include <windows.h>

int main(int argc, char **argv)
{
    char buff[5];
    strcpy(buff,”1234567890″);
    return 0;
}


Yep, that’s a really short program that makes for a buffer overflow.


And yet, “cl /analyze” won’t complain, except to tell you that strcpy is deprecated.


So, what do you do?


The right first step, of course, is to replace strcpy – as a deprecated function, it’s kind of dangerous.


So, let’s say we replace strcpy with strcpy_s, and here’s the output I get from running “cl /analyze”:


Microsoft (R) 32-bit C/C++ Optimizing Compiler Version 14.00.50727.42 for 80×86
Copyright (C) Microsoft Corporation.  All rights reserved.


test.cpp
Microsoft (R) Incremental Linker Version 8.00.50727.42
Copyright (C) Microsoft Corporation.  All rights reserved.


/out:test.exe
test.obj


So, clearly, we’re still not detecting the overflow.


“Isn’t strcpy_s safe? Do we care that it’s not detecting the overflow?”


Sure, it’s safe – but it really depends on what you call ‘safe’. Using strcpy_s like that will simply kill your process right there, with an exception:


—————————
test.exe – Application Error
—————————
The exception unknown software exception (0xc000000d)
occurred in the application at location 0x0040108f.
 


Click on OK to terminate the program
Click on CANCEL to debug the program
—————————
OK   Cancel  
—————————


[So, you did know you can press Ctrl-C in most dialogs to get the text of the message in the clipboard, right? Try it next time you need to report a dialog box error - much better than sending a graphic!]


But if you ship an application that does this, you’ll get a reputation for shipping crap.


Possibly deservedly.


What to do? I’ve known some authors who handle this by catching, and then ignoring, the exception, in somewhere like their main loop.


Quite simply, that’s the wrong thing to do – it may even be the worst thing to do. Why? Because the exception you ignore may be the test packet that gets thrown at you prior to the successful exploit. You definitely want to fail on unexpected exceptions – and a buffer overflow, even when it’s detected, should not be expected.


But, you say, this exception is expected, and will be handled in some other way – say, by telling the user that he’s entered a bad value, and requesting his input again.


Well then, it’s not an exception, and you shouldn’t use a function that makes an exception out of a commonplace occurrence. Look instead to StringCchCopy, which will return an error result when you overflow. Handle the error result correctly, and you avoid the mess and overhead of exception handling. Or, use strncpy_s with the “_TRUNCATE” parameter for the count value, and get a similar kind of handling.

2 Responses to "Steam will save the world"

  • mathew says:

    strcpy_s? That’s one of the dumbest ideas I’ve seen in a long time. I can just imagine the thinking that must have gone into it…

    “So, what are we going to do about this buffer overflow problem?”

    “Well, it happens because people use things like strcpy, rather than using strncpy like they should and actually checking for error conditions.”

    “Hmm. So, how about we force them to check for error conditions and handle them? Or provide a replacement string implementation that doesn’t have fixed buffer sizes?”

    “What do you think this is, Java? Objective-C on a Mac? Some kind of pinko high level language?”

    “OK, OK. Let’s just put a Band-Aid over the gaping wound. Let’s define strcpy_s, which is exactly as broken as strcpy, except your program crashes immediately rather than becoming corrupt or crashing later on.”

    “But… couldn’t we do that for all strcpy calls and still be within the ANSI spec?”

    “Yes, but that way people would be forced to fix their broken code. If we do it the half-assed way, all the broken code will continue to work.”

    “But we won’t get actual security until every single strcpy call is replaced with strcpy_s.”

    “Well, that’s a Simple Matter Of Programming!”

    “Excellent! Truly we are delivering quality software!”

  • alunj says:

    “Well, it happens because people use things like strcpy, rather than using strncpy like they should and actually checking for error conditions.”

    I’m going to have to disagree with you here – strncpy isn’t a solution, either. The count parameter of strncpy is described (in various ways) as the number of characters from the source string that should be copied, rather than the space that’s available in the destination string. That you can use this parameter, and the min function, to achieve those ends, doesn’t make strncpy a universally good replacement. [The better answer is to create a better string and/or buffer class, and use it, IMHO - but be sure you get it right!]

    “Hmm. So, how about we force them to check for error conditions and handle them? Or provide a replacement string implementation that doesn’t have fixed buffer sizes?”

    Both of these are easily achievable in C++.

    Forcing the check for error conditions comes with the use of SAL and the /analyze compiler switch, which will flag as warnings any place where you do not check a return value that is declared by SAL to be important. Oh, and you do compile with “all warnings are fatal errors”, don’t you?

    As for the replacement string implementations, you have a number of different string classes for various needs, from the simple BSTR through MFC / ATL’s CString, and the STL’s string classes. I’m sure there are more if you look hard enough.

    My goal in writing this post, as you probably figured out, is to ask how I can use the tools provided to find and fix the broken code. I don’t buy the idea that developers can suddenly become super-vigilant and catch themselves from using strcpy badly – and I don’t think that’s what you were saying – I’d just like the tools to find the code in compilation, rather than in test. SAL and /analyze hold out the promise of being better at this over time.

Leave a Reply

Your email address will not be published. Required fields are marked *

You may use these HTML tags and attributes: <a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <strike> <strong>