MS08-014, CVE 2008-0081, addresses a vulnerability in Excel whose root cause is an uninitialized stack variable.  You probably have seen these types of compiler warnings before:

C:\temp>cl stack.cpp
Microsoft (R) 32-bit C/C++ Optimizing Compiler Version 15.00.21022.08 for 80x86 Copyright (C) Microsoft Corporation.  All rights reserved.

stack.cpp
c:\temp\stack.cpp(49) : warning C4700: uninitialized local variable 'pNoInit' used ...

The code still compiles and links fine so maybe you have even ignored this warning in the past.  This type of code defect is not discussed as widely as buffer overruns but MS08-014 is a good example of why using uninitialized variables is potentially dangerous.  We'll use the following PoC code to illustrate the vulnerability, show how it is vulnerable to attack, and then give more detail about these compiler warnings:

// stack.cpp
unsigned char scode[] = "shell code";

void parse();

void p1();
void p2();

int main(int argc, char* argv[])
{
    parse();
    return 0;
}

void parse()
{
    p1();
    p2();
}

void p1()
{
    // Assume p1 is reading data from the file to s
    // Content of s could be controlled by the attacker.

    // For simplicity, assign s to 0x0013fee8, location of return address to stack!main+0x8

    int s[64];
    for (int i=0; i<64; i++)
    {
        s[i] = 0x0013fee8;                 // spray the stack buffer
    }
}

void p2()
{
    // Assume p2 is the next parsing function to read data.
    // For example, it tries to load an image from the file, and assign to its field.
    // If the content image is the shellcode, by modifying the return address in the
    // stack, the shell code runs.

    struct {
        char * pImage;
} *pNoInit;

    // This is to align the stack layout to demonstrate the attack
    char s[100];

    pNoInit->pImage = (char *) scode;
}

pNoInit is not initialized before it is used in function p2().  Is this a programming mistake that could lead to code execution?  The answer lies in whether the attacker can prepare the stack right before p2() is called.  If so, the value of pNoInit would be controlled by the attacker.

In our example, function p1() is called right before p2(). In function p1(), we tried to set values in specific location in the stack. These values would still be kept in the stack, even when p1 returns. Therefore, when p2 enters, the attacker controls the value of pNoInit. In this case, it points to the return address of the main(), and off we go.

Here's what it looks like in the debugger:

stack!p2:
00401090 55              push    ebp
0:000> k
ChildEBP RetAddr
0013fed4 0040101d stack!p2 [h:\work\stack\stack\stack.cpp @ 57] 0013fedc 00401008 stack!parse+0xd [h:\work\stack\stack\stack.cpp @ 33]
0013fee4 004012ae stack!main+0x8 [h:\work\stack\stack\stack.cpp @ 26] 0013ffc0 7c816fd7 stack!mainCRTStartup+0x173 [f:\vs70builds\3077\vc\crtbld\crt\src\crt0.c @ 259]
WARNING: Stack unwind information not available. Following frames may be wrong.
0013fff0 00000000 kernel32!RegisterWaitForInputIdle+0x49

0:000> dv
        pNoInit = 0x0013fee8
              s = char [100] "???"

stack!p2+0x32:
004010c2 8b458c          mov     eax,dword ptr [ebp-74h] ss:0023:0013fe60=0013fee8

0:000> t
eax=0013fee8 ebx=7ffdf000 ecx=00000063 edx=7c90eb63 esi=00000a28 edi=00000000
eip=004010c5 esp=0013fe5c ebp=0013fed4 iopl=0         nv up ei pl zr na pe nc
cs=001b  ss=0023  ds=0023  es=0023  fs=003b  gs=0000             efl=00000246
stack!p2+0x35:
004010c5 c70030704000    mov     dword ptr [eax],offset stack!scode (00407030) ds:0023:0013fee8=004012ae

0:000> k
ChildEBP RetAddr
0013fed4 0040101d stack!p2+0x3b [h:\work\stack\stack\stack.cpp @ 79] 0013fedc 00401008 stack!parse+0xd [h:\work\stack\stack\stack.cpp @ 33]
0013fee4 00407030 stack!main+0x8 [h:\work\stack\stack\stack.cpp @ 26] 0013ffc0 7c816fd7 stack!scode
WARNING: Stack unwind information not available. Following frames may be wrong.

When the compiler warned us about the uninitialized variable above, it was attempting to save us from a real vulnerability!

You can also get more help from the compiler by using the /W4 switch. Check the following test.cpp:

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

        if (argc < 3)
        {
            pMaynotInit = 0;
        }

        return *pNoInit + *pMaynotInit;
}

C:\test>cl /W4 test.cpp
Microsoft (R) 32-bit C/C++ Optimizing Compiler Version 13.10.3077 for 80x86
Copyright (C) Microsoft Corporation 1984-2002. All rights reserved.

test.cpp
c:\test\test.cpp(11) : warning C4700: local variable 'pNoInit' used without having been initialized
c:\test\test.cpp(11) : warning C4701: local variable 'pMaynotInit' may be used without having been initialized

Now we see the C4700 warning, same as before, and also a C4701 warning, "potentially" uninitialized local variable.  /W4 will generate a large number of warnings (which is why most people don't compile with /W4) however most 4701 warnings are trivially fixed. Those additional warnings may be noise or they might be an indicator of a subtle bug.

For example, the following code will be flagged as a potential use of an uninitialized variable:

MYOBJECT *p;
HRESULT hr = S_OK;
          
if(SUCCEEDED(hr))    
{ 
     hr = MyInitializationFunction(&p);     
} 

if(SUCCEEDED(hr))    
{ 
     p->DoSomething();    /*flagged as potential uninitialized use*/    
}

The compiler doesn’t know that MyInitializationFunction() will initialize *p on success. However initializing p to NULL on declaration avoids any ambiguity – and is good practice just in case there is a bug in MyInitializationFunction leading to it returning success without initializing its parameter!

The other common source of noise from the 4701 warning is:

foo(BYTE packet_type)
{
MYOBJECT *p;
switch(packet_type)
{
       case REQUEST:
              p= new MYOBJECT ...;

       case RESPONSE:
              p= new MYOBJECT ...;

}
p->DoSomething();    <- flagged as potential uninitialized use
...

The compiler is picking up on the theoretical code path where packet_type is neither REQUEST nor RESPONSE. The implicit ‘default’ case of the switch statement does not initialize p - hence the warning. Maybe there really are only two packet types that can reach this code (eg because validation of packet_type happens before this function is ever called). If so then one of the switch statements will always be executed and p will always be initialized. Once again however the warning is easily fixed by initializing p to null on declaration – and in the event that the code above can be reached (e.g. via a malicious attacker crafting a network packet with a bogus type field) then we’ve reduced an almost certain code execution vulnerability to a null dereference. Of course the better way to fix this is to add an explicit default statement that returns an error code when it finds an unexpected packet_type value.

We hope the above discussion shows that these compiler warnings should be treated seriously.  In fact, we're hoping to get them added as a banned warning type in the next version of the SDL.

April 16, 2008 Update: Revised code to be more accurate.

- Security Vulnerability Research & Defense Bloggers

*Postings are provided "AS IS" with no warranties, and confers no rights.*