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:

 

https://secunia.com/advisories/13965/

https://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

• Anonymous
  February 03, 2005
  The comment has been removed
• Anonymous
  February 03, 2005
  Totally true - but wait for my next post on the subject :)
• Anonymous
  February 03, 2005
  Oooh, sounds exciting! :)
• Anonymous
  February 03, 2005
  The comment has been removed
• Anonymous
  February 04, 2005
  Totally agree - but you'd be AMAZED at the number of the people who use C++ as a better-C!
• Anonymous
  February 05, 2005
  The comment has been removed
• Anonymous
  February 06, 2005
  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."
• Anonymous
  February 06, 2005
  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.
• Anonymous
  February 06, 2005
  The comment has been removed
• Anonymous
  February 06, 2005
  The comment has been removed
• Anonymous
  February 06, 2005
  The comment has been removed
• Anonymous
  February 07, 2005
  "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?
• Anonymous
  February 07, 2005
  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!
• Anonymous
  February 08, 2005
  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.
• Anonymous
  February 15, 2005
  The comment has been removed
• Anonymous
  February 16, 2005
  The comment has been removed