Security Stuff in Whidbey – More Secure Buffer Function Calls: AUTOMATICALLY!


In my previous blog I very briefly touched on the new C runtime library added to Whidbey.


 


Take a look at the following simple code:


 


int main(int argc, char* argv[]) {


    char t[10];


       


    if (2==argc)


strcpy(t,argv[1]);


 


      


    return 0;


}


 


As you can see there is a bug on the third line, the code is copying untrusted input to a stack-based buffer. A classic “Stack Smash.” On Windows, this would generally not be a security bug as the code can only run as you. But if this were in an elevated process, such as a setuid root application on Linux, Mac OS X or Unix, then this would be a bad security bug requiring a fix. Here are a couple of examples:


 


http://secunia.com/advisories/13965/


http://secunia.com/advisories/12481/


 


And just to show you the compiler is doing the ‘right’ thing by generated code ‘as implemented’ here’s a snippet of the ASM file generated by the compiler:


 


; 9    :    char t[10];


; 10   :    strcpy(t,argv[1]);


 


      mov   eax, DWORD PTR _argv$[esp+12]


      mov   eax, DWORD PTR [eax+4]


      lea   edx, DWORD PTR _t$[esp+16]


      sub   edx, eax


      npad  6


$LL3@main:


      mov   cl, BYTE PTR [eax]


      mov   BYTE PTR [edx+eax], cl


      add   eax, 1


      test  cl, cl


      jne   SHORT $LL3@main


 


The loop from $LL3@main is an inline strcpy. Trust me!


 


A more secure version would look a little like this:


 


int main(int argc, char* argv[]) {


    char t[10];


      


    if (2==argc)


strncpy(t,argv[1],10);


 


      


    return 0;


}


 


Of better yet, you could use the Safer CRT:


 


int main(int argc, char* argv[]) {


    char t[10];


       


    if (2==argc)


strcpy_s(t,_countof(t)argv[1]);


 


       


    return 0;


}


 


Now let’s say for a moment that you don’t change the function call from strcpy to strcpy_s, because you either missed it, or simply don’t want to tinker with the source too much. There’s a cool new feature for C++ code in Whidbey. If you add the following to your stdafx.h header:


 


#define _CRT_SECURE_CPP_OVERLOAD_STANDARD_NAMES 1


 


Recompile the code, and then look at the ASM file again:


 


; 9    :    char t[10];


; 10   :    strcpy(t,argv[1]);


 


      mov   eax, DWORD PTR [eax+4]


      push  eax


      lea   ecx, DWORD PTR _t$[esp+20]


      push  10                            ; 0000000aH


      push  ecx


      call  DWORD PTR __imp__strcpy_s


 


Well lookie here – the compiler knows the size of the destination buffer, so it automatically replaces the strcpy with a strcpy_s.


 


Very cool stuff… :)

Comments (16)

  1. Dean Harding says:

    This isn’t a magic bullet, though. If you had something like this:

    void MyUnsafeMethod(char *dest, char *src)

    {

    strcpy(dest, src);

    }

    void main(int argc, char **argv)

    {

    char t[10];

    if (argc > 1)

    MyUnsafeMethod(t, argv[1]);

    }

    (Obviously, that’s a silly example, but anyway…) in this case, the compiler can’t know the size of the "dest" buffer in MyUnsafeMethod, so you’d still need to modify the code to call strcpy_s yourself!

  2. Michael Howard says:

    Totally true – but wait for my next post on the subject :)

  3. Dean Harding says:

    Oooh, sounds exciting! :)

  4. Anonymous says:

    But if I’m writing in C++ I wouldn’t need this anyway:

    std::string t;

    if (2==argc) t = argv[1];

    I find it difficult to get excited about these new C functions. Why would I be calling low level functions like strcpy anyway, when there are simpler and safer high level strings easily available?

  5. Michael Howard says:

    Totally agree – but you’d be *AMAZED* at the number of the people who use C++ as a better-C!

  6. Dude…

    > On Windows, this would generally not be a security bug as the

    > code can only run as you. But if this were in an elevated process,

    > such as a setuid root application on Linux, Mac OS X or Unix, then

    > this would be a bad security bug requiring a fix.

    Don’t tarnish your excellent reputation as someone who knows a thing or two about secure coding by posting crap like this. You know better than this.

    I didn’t think this was supposed to be a stupid marketing blog. The implication that this doesn’t need fixing because the code runs on Windows is irresponsible and idiotic. If that were true, why would the Whidbey team bother with the safer CRT to start with?

    http://secunia.com/advisories/13802/

    http://secunia.com/advisories/13464/

  7. Anonymous says:

    Please correct me if I’m wrong but as far as I know quite a lot of users use Windows as an admin user. Because of this I find the following statement misleading:

    "On Windows, this would generally not be a security bug as the code can only run as you."

  8. Michael Howard says:

    It’s not misleading at all – where’s the vuln? You’re a user, or an admin, and you can run code as you!! If you were a user, and *this* was code running in elevated privilege, then yeah, it’d be a security bug. But we don’t have that notion in Windows. An arbitrary app (ie; this code) runs as YOU.

  9. Michael Howard says:

    >>Don’t tarnish

    >>The implication that this doesn’t need fixing

    Yikes!! I never said this wouldn’t be fixed!! What I said is this is a code quality bug, we have *NO* notion of Setuid root in Windows, so in most cases this would not be a security defect, a code quality bug, yes. Now, if this is running in service running in system context, and you can get a command-line arg in there (you need to be an admin to do it by default) then yeah, it would be a security bug!

  10. Dean Harding says:

    I think the anonymous people are missing the point that *this exact piece of code* is not too big a deal, not buffer overruns in general. Passing mangled data via the command-line to an application in order to exploit a buffer overrun isn’t going to get you anything at all under Windows.

    It’s a different story if you can exploit a buffer overrun in IIS or some service that’s running with higher privileges than you are. But when you can only run applications from the command-line as yourself, then you don’t gain anything by exploiting a buffer overrun (because you can just execute an app you wrote yourself if you want to do that).

    Buffer overruns *in general* are bad and need to be fixed (hence all the effort in Whidbey, I guess) but *this exact example* is not a *security* bug on Windows (it’s still a bug of course, cause it can still crash your app).

  11. Michael Howard says:

    And for anonymous’ interest:

    >>http://secunia.com/advisories/13802/

    this was not a BO reading argv from the command line.

    >>http://secunia.com/advisories/13464/

    and neither was this!!!

  12. "On Windows, this would generally not be a security bug as the code can only run as you."

    Huh? So if notepad did something like this when opening a txt file, there’d be no harm, right?

    It’s a vulnerability in the app that could allow an attacker to compromise you by giving you bad data. Or am I missing the point?

  13. Michael Howard says:

    If notepad had this – and clicking on a TXT file causes notepad to BO, then yeah, that would be a security bug. But I’m talking about the code sample I showed. Nothing else!

  14. In your much anticipated next post on this, it might be nice to point out what, if any, impact this might have on PREfast output. Would seem cool if PREfast could distinguish between code like this that gets automagically fixed and the code that can’t be as Dean pointed out.

    And I find it interesting that this automatic fixing of buffer overflows still might break a string’s syntax, if the char string in question is a utf-8 or other encoding that might have a trail byte missing. I’ve seen code that overflows from one buffer into another that happens to be benign, where fixing it in such an automated fashion would introduce a new truncation problem.

  15. Well, this could be a security bug too, if it was a shell registered app. The filename of the item it is invoked on can be passed as the second arg, right? So a specially malformed filename could execute code?

  16. JD says:

    Mike, the backhanded swipe at Linux was ill-advised.

    There is nothing inherent in Windows that makes a stack smash less dangerous than on Linux. You mention setuid as if it is relevant here, then brush aside concern about notepad.exe because you can’t imagine this code sample being used on untrustworthy data?? Please, you’re better than that.

    Windows has processes that run with high privileges, by default most users have high privileges (admin) even in common corporate scenarios (um, like at Microsoft), and this is much more common than setuid.

    And running this application against untrustworthy data is unlikely… why? How can you tell how users are using this hypothetical program? Why would they run it with setuid on Linux? Why wouldn’t they run it against untrustworthy data?

    "But I’m talking about the code sample I showed. Nothing else!" — except that you brought up Linux setuid, which has nothing to do with your code sample?

    The point? The backhanded comment about Linux was misplaced, and doesn’t help your point. At best it’s a jibe, at worst it’s FUD. honestly, I expected more.