10

Visual Studio 2013 C++ projects have a /GS switch to enable buffer security check validation at runtime. We are encountering many more STATUS_STACK_BUFFER_OVERRUN errors since upgrading to VS 2013, and suspect it has something to do with improved checking of buffer overrun in the new compiler. I've been trying to verify this and better understand how buffer overrun is detected. I'm befuddled by the fact that buffer overrun is reported even when the memory updated by a statement only changes the contents of another local variable on the stack in the same scope! So it must be checking not only that the change doesn't corrupt memory not "owned" by a local variable, but that the change doesn't affect any local variable other than that allocated to the one referenced by the individual update statement. How does this work? Has it changed since VS 2010?

Edit: Here's an example illustrating a case that Mysticial's explanation doesn't cover:

void TestFunc1();

int _tmain(int argc, _TCHAR* argv[])
{
   TestFunc1();
   return 0;
}

void TestFunc1()
{
   char buffer1[4] = ("123");
   char buffer2[4] = ("456");
   int diff = buffer1 - buffer2;
   printf("%d\n", diff);
   getchar();
   buffer2[4] = '\0';
}

The output is 4 indicating that the memory about to be overwritten is within the bounds of buffer1 (immediately after buffer2), but then the program terminates with a buffer overrun. Technically it should be considered a buffer overrun, but I don't know how it's being detected since it's still within the local variables' storage and not really corrupting anything outside local variables.

This screenshot with memory layout proves it. After stepping one line the program aborted with the buffer overrun error. Debugger screenshot with memory layout

I just tried the same code in VS 2010, and although debug mode caught the buffer overrun (with a buffer offset of 12), in release mode it did not catch it (with a buffer offset of 8). So I think VS 2013 tightened the behavior of the /GS switch.

Edit 2: I managed to sneak past even VS 2013 range checking with this code. It still did not detect that an attempt to update one local variable actually updated another:

void TestFunc()
{
   char buffer1[4] = "123";
   char buffer2[4] = "456";
   int diff;
   if (buffer1 < buffer2)
   {
      puts("Sequence 1,2");
      diff = buffer2 - buffer1;
   }
   else
   {
      puts("Sequence 2,1");
      diff = buffer1 - buffer2;
   }

   printf("Offset: %d\n", diff);
   switch (getchar())
   {
   case '1':
      puts("Updating buffer 1");
      buffer1[diff] = '!';
      break;
   case '2':
      puts("Updating buffer 2");
      buffer2[diff] = '!';
      break;
   }
   getchar(); // Eat enter keypress
   printf("%s,%s\n", buffer1, buffer2);
}
BlueMonkMN
  • 25,079
  • 9
  • 80
  • 146
  • 3
    I *believe* one of the things it does is to insert dummy data adjacent to stack objects and checks them whenever convenient (such as exiting the function). If the data changed then it knows something corrupted it. That's just my guess though. – Mysticial Sep 10 '15 at 21:40
  • @Mysticial that wouldn't explain my test case where I calculated the offset between the buffers for two local variables to be adjacent and yet the overrun updating the first was still detected. – BlueMonkMN Sep 10 '15 at 22:10
  • 1
    Show an example of what you mean, including output of the variable addresses. I'm fairly sure Mystical is right. – Mats Petersson Sep 10 '15 at 22:14
  • @MatsPetersson I've added proof that something more must be going on. – BlueMonkMN Sep 11 '15 at 12:56
  • In VS2013, Win32, Debug, I see an offset of 12, gap filled with `0xcc`. – Henrik Sep 11 '15 at 13:12
  • @Henrik You have to run in Release mode to see the buffer offset of 4. The question is how the overrun is caught in this case. – BlueMonkMN Sep 11 '15 at 13:18
  • In release build, index out of range access is detected at compile time and in the disassembly there's just `call __report_rangecheckfailure`. – Henrik Sep 11 '15 at 13:19
  • @Henrik How did you determine there was an out of range detected at compile time? Also, it this something new with VS 2013? Why was it not caught by VS 2010? – BlueMonkMN Sep 11 '15 at 13:21
  • Just right click on the line `buffer2[4] = 0;` and select "Goto Disassembly". I don't know how this was handled in VS2010. I usually try to avoid raw arrays. – Henrik Sep 11 '15 at 13:23

2 Answers2

2

You are seeing an improvement to the /GS mechanism, first added to VS2012. Originally /GS could detect buffer overflows but there's still a loop-hole where attacking code can stomp the stack but bypass the cookie. Roughly like this:

void foo(int index, char value) {
   char buf[256];
   buf[index] = value;
}

If the attacker can manipulate the value of index then the cookie doesn't help. This code is now rewritten to:

void foo(int index, char value) {
   char buf[256];
   buf[index] = value;
   if (index >= 256) __report_rangefailure();
}

Just plain index checking. Which, when triggered, instantly terminates the app with __fastfail() if no debugger is attached. Backgrounder is here.

Hans Passant
  • 922,412
  • 146
  • 1,693
  • 2,536
  • If you look at my sample code under edit 2, interestingly, this did not trigger the buffer overflow. Reading your link closely I realized that it was because the overflow check is triggered specifically on NULL ('\0') characters! Interesting, and thank you! – BlueMonkMN Sep 11 '15 at 14:27
  • It seems like many such cases could be reported at compile time. Is there any way to enable reporting of some of these cases at compile time? – BlueMonkMN Sep 11 '15 at 14:54
1

From the MSDN page on /GS in Visual Studio 2013 :

Security Checks

On functions that the compiler recognizes as subject to buffer overrun problems, the compiler allocates space on the stack before the return address. On function entry, the allocated space is loaded with a security cookie that is computed once at module load. On function exit, and during frame unwinding on 64-bit operating systems, a helper function is called to make sure that the value of the cookie is still the same. A different value indicates that an overwrite of the stack may have occurred. If a different value is detected, the process is terminated.

for more details, the same page refers to Compiler Security Checks In Depth:

What /GS Does

The /GS switch provides a "speed bump," or cookie, between the buffer and the return address. If an overflow writes over the return address, it will have to overwrite the cookie put in between it and the buffer, resulting in a new stack layout:

  • Function parameters
  • Function return address
  • Frame pointer
  • Cookie
  • Exception Handler frame
  • Locally declared variables and buffers
  • Callee save registers

The cookie will be examined in more detail later. The function's execution does change with these security checks. First, when a function is called, the first instructions to execute are in the function’s prolog. At a minimum, a prolog allocates space for the local variables on the stack, such as the following instruction:

sub esp, 20h

This instruction sets aside 32 bytes for use by local variables in the function. When the function is compiled with /GS, the functions prolog will set aside an additional four bytes and add three more instructions as follows:

sub   esp,24h
mov   eax,dword ptr [___security_cookie (408040h)]
xor   eax,dword ptr [esp+24h]
mov   dword ptr [esp+20h],eax

The prolog contains an instruction that fetches a copy of the cookie, followed by an instruction that does a logical xor of the cookie and the return address, and then finally an instruction that stores the cookie on the stack directly below the return address. From this point forward, the function will execute as it does normally. When a function returns, the last thing to execute is the function’s epilog, which is the opposite of the prolog. Without security checks, it will reclaim the stack space and return, such as the following instructions:

add   esp,20h
ret

When compiled with /GS, the security checks are also placed in the epilog:

mov   ecx,dword ptr [esp+20h]
xor   ecx,dword ptr [esp+24h]
add   esp,24h
jmp   __security_check_cookie (4010B2h)

The stack's copy of the cookie is retrieved and then follows with the XOR instruction with the return address. The ECX register should contain a value that matches the original cookie stored in the __security_cookie variable. The stack space is then reclaimed, and then, instead of executing the RET instruction, the JMP instruction to the __security_check_cookie routine is executed.

The __security_check_cookie routine is straightforward: if the cookie was unchanged, it executes the RET instruction and ends the function call. If the cookie fails to match, the routine calls report_failure. The report_failure function then calls __security_error_handler(_SECERR_BUFFER_OVERRUN, NULL). Both functions are defined in the seccook.c file of the C run-time (CRT) source files.

Nikos Athanasiou
  • 29,616
  • 15
  • 87
  • 153
  • As you can see in my newly added sample code and screenshot, I don't think any of what you're describing here would catch the case I saw being caught. – BlueMonkMN Sep 11 '15 at 12:58
  • @BlueMonkMN This is not a VS13 improvement, the buffer overrun will be reported as well in VS2012. In both cases VS uses a range check – Nikos Athanasiou Sep 11 '15 at 15:20
  • Right, the change I'm concerned with was introduced with VS 2012, as indicated by Hans Passant. We upgraded from VS 2010 to VS 2013, so I wasn't sure which version introduced the behavior. – BlueMonkMN Sep 11 '15 at 15:22