Fix Those Buffer Overruns!

  • Article

 

Michael Howard
Microsoft Corporation

Updated May 28, 2002

When David LeBlanc and I defined the table of contents for Writing Secure Code, it was obvious that we had to focus on buffer overruns because so many developers make so many mistakes in their code that lead to exploitable buffer overruns. In this article, I'll focus on what makes them so bad, why they exist, and how to fix them.

Why Buffer Overruns Exist

There are a number of things that need to happen for a buffer overrun to occur, including:

  • Use of a non-type-safe language, such as C/C++.
  • Accessing or copying a buffer in an insecure manner.
  • The compiler places buffers next to or near critical data structures in memory.

Let's look at each event in detail.

Buffer overruns are primarily a C and C++ issue because these languages perform no array bounds checking and no type-safety checking. C/C++ allow the developer to create programs that run very close to the metal, which allows direct access to memory and machine registers. The upshot of this is performance; it's hard to make any application run as fast as a well-written C/C++ application. Buffer overruns exist in other languages, but they are rare. And if such a bug exists it's usually not the fault of the developer, but rather the fault of the run-time environment.

Next, if the application takes data from a user (or an attacker) and copies the data to a buffer maintained by the application with no regard for the destination buffer size, then you may overflow the buffer. In other words, the code allocates N-bytes, and the code copies more than N-bytes to the allocated buffer. Think of it this way—you have a 12-ounce glass, and you pour 16 ounces of water into the glass. Where does the extra 4 ounces of water go? All over the place!

Finally, and most importantly, buffers are often placed next to "interesting" data structures by the compiler. For example, in the case of a function that has a buffer on the stack, the function's return address is placed in memory after the buffer. So, if the attacker can overflow the buffer, he can overwrite the function return address so that when the function returns, it returns to an address determined by the attacker. Other interesting data structures include C++ v-tables, exception handler addresses, function pointers, and so on.

Ok, I've waffled on, so let's look at an example.

What's wrong with this code?

void CopyData(char *szData) {
   char cDest[32];
   strcpy(cDest,szData);

   // use cDest
   ...
}

Surprisingly, there may be nothing wrong with this code! It all depends on how CopyData() is called. For example, the following code is safe:

char *szNames[] = {"Michael","Cheryl","Blake"};
CopyData(szName[1]);

The code is safe because the names are hard-coded, and it is known that each string does not exceed 32 characters in length, hence the call to strcpy is always safe. However, if the sole argument to CopyData, szData, comes from an untrusted source, such as a socket or a file, then strcpy will copy the data until it hits a null character, and if the data is greater than 32 characters in length, the cDest buffer is overrun and any data above the buffer in memory is clobbered. Unfortunately, in this case the clobbered data is the return address from CopyData, which means when CopyData finishes, it continues execution at a location dictated by the attacker. Bad!

Other data structures are sensitive too. Imagine if the v-table of a C++ class were corrupted in code like this:

void CopyData(char *szData) {
   char cDest[32];
   CFoo foo;
   strcpy(cDest,szData);

   foo.Init();
}

This example assumes that class CFoo has virtual methods, as well as a v-table or a list of addresses for the class methods common to all C++ classes. If the v-table is damaged by overwriting the cDest buffer, then any virtual method on the class, Init() in this example, may call an address dictated by the attacker rather than Init(). By the way, if you think your code is safe if no C++ method is called, one method is always called—the class' virtual destructor! Of course, if you have a class that makes no method calls, you should ask why the class exists.

Fixing Buffer Overruns

Now let's move onto something a little more positive—how to remove and prevent buffer overruns in your code.

Migrating to Managed Code

In February and March 2002, we held the Microsoft Windows® Security Push. During this time, my group trained over 8,500 people in what it takes to design, write, test, and document-secure features. One recommendation we made to all designers was to come up with plans for moving appropriate applications and tools from native Win32® C++ code to managed code. We did this for a number of reasons, but the main one was to help mitigate buffer overruns. In managed code, it's much harder to create code that contains a buffer overrun because the code you write does not have direct access to pointers, machine registers, or memory. You should consider, or at least plan for, migrating certain applications and tools to managed code. For example, a management tool is a perfect candidate for migration. Obviously, you need to be realistic; you're not going to migrate all your applications in an evening from C++ to C# or another managed language.

Follow the Golden Rules

When writing C and C++ code, you should be careful about how you manage data from users. Follow these rules if you have a function that takes a buffer from an untrusted source:

  • Require that the code pass the buffer length.
  • Probe the memory.
  • Be defensive.

Let's look at each one of these bullets in detail.

Require the code pass the buffer length

You have a bug if you have any function call that has a signature like this:

void Function(char *szName) {
   char szBuff[MAX_NAME];
   // Copy and use szName
   strcpy(szBuff,szName);
}

The problem with this code is the function has no idea how long szName is, which means you cannot safely copy the data. The function should take the size of the szName:

void Function(char *szName, DWORD cbName) {
   char szBuff[MAX_NAME];
   // Copy and use szName
   if (cbName < MAX_NAME)
      strncpy(szBuff,szName,MAX_NAME-1);
}

However, you should not necessarily trust cbName. The attacker could set the name and the buffer size, so you need to check!

Probe the memory

How do you know szName and cbName are valid? Do you trust the user to give you valid values? Generally, the answer is no. One simple way to confirm that the buffer size is valid is to probe the memory. The following code snippet shows how you can do this in a debug version of your code:

void Function(char *szName, DWORD cbName) {
   char szBuff[MAX_NAME];
   
#ifdef _DEBUG

   // Probe
   memset(szBuff, 0x42, cbName);
#endif

   // Copy and use szName
   if (cbName < MAX_NAME)
      strncpy(szBuff,szName,MAX_NAME-1);
}

This code will attempt to write the value 0x42 to the destination buffer. You're probably wondering why you would do this rather that just copy the buffer. By writing a fixed, known value to the end of the destination buffer, you can force the code to fail if the source buffer is too big. It can also find development bugs early in the development process. It's better to fail rather than run the attacker's malicious payload, and that's why you don't want to copy the attacker's buffer.

Note You should only do this in a debug build to help catch buffer overruns during test.

Be defensive

In all honesty, probing is useful, but it does not make you safe from attack. The only real way to be safe is to code defensively. You'll note the code is already defensive. It checks that the data coming into the function is no larger than the internal buffer, szBuff. However, certain functions have potentially serious security issues if misused when manipulating or copying untrusted data. The critical point here is untrusted data. When reviewing your code for buffer overrun bugs, you should follow the data as it flows through the code and question the assumptions about that data. It's amazing what bugs you'll find when you realize that some of the assumptions are incorrect.

Some of the functions to watch out for include classic functions such as strcpy, strcat, gets, and so on. But don't rule out the so-called safe n-versions of strcpy and strcat—strncpy and strncat. These functions are supposedly more secure and safer to use because they allow the developer to limit the size of the data copied into the destination buffer. However, developers get these wrong too! Take a look at the code below. Can you see the flaw?

#define SIZE(b) (sizeof(b))
char buff[128];
strncpy(buff,szSomeData,SIZE(buff));
strncat(buff,szMoreData,SIZE(buff));
strncat(buff,szEvenMoreData,SIZE(buff));

If you need a clue, look at the last arguments to each string handling function. Give up? Before I give you the answer, I often joke that if you banned the "unsafe" string handling functions, and mandated the safer n-versions, you'd spend the rest of your life fixing the newly introduced bugs. Here's why. First, the last argument is not the total size of the destination buffer. It's how much space is left in the buffer, and each time the code adds to buff, buff is essentially getting smaller. The second issue is even when people pass in the size of the buffer, they are often off-by-one. Do you include the trailing null in the string size calculation or not? When I poll audiences on this it's usually a 50-50 split. One half of the room thinks you do accommodate the trailing null when calculating the buffer size, and the other half thinks not. Third, in some cases the n-version may not null-terminate the resulting string, so make sure you read the documentation.

If you write C++ code, consider using the ATL, STL, MFC, or your favorite string handling classes to manipulate strings rather than handling the bytes directly. The only potential downside is possible performance degradation, but generally speaking, using most of these classes leads to more robust and maintainable code.

Compile with /GS

This new compile-time option in Visual C++® .NET inserts values into the stack-frames of certain functions to help mitigate the potential vulnerability of stack-based buffer overruns. Keep in mind that this option does not cure your code, nor does it remove any bugs. It merely acts as a backstop to help reduce the potential of certain classes of buffer overrun from becoming exploitable buffer overruns, allowing the attacker to inject code into your process and execute it. Think of it as a very small insurance policy. Note that for new native Win32 C++ projects created using the Win32 Application Wizard, the default is to enable this option. Also, Windows Server 2003 is compiled with this option. For more information, refer to Brandon Bray's Compiler Security Checks In Depth.

Spot the Vulnerability

I want to wrap up with some code that has at least one security flaw in it. Can you see it? I'll give you the answers in my next article!

WCHAR g_wszComputerName[INTERNET_MAX_HOST_NAME_LENGTH + 1];

// Get the server name and convert it to the Unicode string.
BOOL GetServerName (EXTENSION_CONTROL_BLOCK *pECB) {
   DWORD   dwSize = sizeof(g_wszComputerName);
   char    szComputerName[INTERNET_MAX_HOST_NAME_LENGTH + 1];

   if (pECB->GetServerVariable (pECB->ConnID,
            "SERVER_NAME",
            szComputerName,
            &dwSize)) {
   // rest of code snipped

Michael Howard is a Security Program Manager in the Secure Windows Initiative group at Microsoft and is the coauthor of Writing Secure Code. His main focus in life is making sure people design, build, test, and document nothing short of a secure system. His favorite line is "One person's feature is another's exploit."