Safe Integer Arithmetic in C


There has been plenty of literature written regarding integer arithmetic issues and security bugs. If you need a good refresher, I would urge you to read one or more of the following:

 

 

Recently, the good folks in the Windows division set out to create small set of C functions (obviously, you can use them in C++ code too) to perform safe integer arithmetic. These functions are used mainly to perform safe memory allocation calculations and array offset calculations. The small library is aptly called IntSafe. Not to be confused with SafeInt from LeBlanc, IntSafe is simply a set of easy to use function calls, while SafeInt is a C++ template class.

 

IntSafe has two categories of functions, those that perform real math, and those that convert from one data type to another. The math functions are listed here, and the conversion functions are listed here.

 

Now here’s something you should know if you want to use IntSafe. All arithmetic is performed on unsigned values. If you want to use signed values, then use the appropriate conversion function first to convert the value to an unsigned value safely. These are omitted on purpose – we found that it was too easy to use the safe functions and assume nothing bad could ever happen because, well, you are using the safe functions! Also, we found that many of the serious security bugs were signed #’s were involved could be fixed by simply switching to unsigned values. Why use a signed value to index an array? Or a signed value to allocate or reallocate memory?

 

 For example, the following code converts a signed int to an unsigned long:

 

#include “intsafe.h”

 

 

int cElem = GetValueFromTheNetwork();

unsigned long ulcElem = 0L;

if (SUCCEEDED(IntToULong(cb,&ulcb))) {

      // success

}

 

If IntToULong is passed a negative value, the function returns INTSAFE_E_ARITHMETIC_OVERFLOW, and fails. Simply casting a negative number to an unsigned value is dangerous, so don’t take any risks.

 

There are plenty more functions like this for converting from one data type to another.

 

Next, we want to perform the actual math operation. There are three categories of math: addition, subtraction and multiplication. There are no division functions, because you can’t overflow a division! Sure you can divide by zero, but you know to check for that in your code, right? Actually, there are some corner cases when using negative numbers, but IntSafe handles only zero and positive numbers.

 

This code performs a data conversion and then multiplies the two numbers together, and then adds one to the result:

 

int cElem = GetValueFromTheNetwork();

DWORD cbElem = 0, cbBlockSize = 0x00F00000;

void *p = NULL;

 

// Performing: cbElem = (DWORD)cElem * cbBlockSize + 1

if (SUCCEEDED(IntToDWord(cElem,&cbElem)) &&

    SUCCEEDED(DWordMult(cbBlockSize, cbElem, &cbElem)) &&

    SUCCEEDED(DWordAdd(cbElem,1,&cbElem))) {

 

      // Cool! Calculation worked

      p = malloc(cbElem);

      assert(p);

}

 

if (p) {

      // use p

      free(p);

 

} else {

      // Oops! Overflow, or out of memory

}

 

So what about kernel mode code? Well, there is a kernel-mode header also, named ntintsafe.h. Rather than returning an HRESULT, these functions return an NTSTATUS, and the functions are prefixed with Rtl. Other than these little differences, the code is basically the same between the user mode and kernel mode functions.

 

I want to point out that these functions are being used a great deal in Windows Vista, primarily in code that calculates buffer sizes and array offsets. I wouldn’t use them for all arithmetic, just when you’re calculating an index, buffer size or computing some other form of bound check.

 

Finally, these functions are optimized for speed — they are inline on purpose, and the compiler generates amazingly good code.

 

You can pick up these headers in the latest Windows Server 2003 and Windows Vista Platform SDKs, or to save you the hassle, you can get them here too :)

 

<Special thanks to Reiner Fink for creating IntSafe and for reviewing drafts of this posting and thanks to Raymond Chen, Larry Osterman and Daniel Wang for also reviewing the drafts>

intsafe.zip

Comments (10)

  1. Selva says:

    Hi Howard,

    I have a very dumb question on the code snippet which was used to demonstrate integer overflow and underflow in the MSDN article

    "Reviewing Code for Integer Manipulation Vulnerabilities"

    Thanks for the article and I am really enjoying reading it.But I have one doubt:

    I think I am not getting the overflow scenario correctly.

    "

    bool func(char *s1, size_t len1,

    char *s2, size_t len2) {

    if (1 + len1 + len2 > 64)

    return false;

    // accommodate for the trailing null in the addition

    char *buf = (char*)malloc(len1+len2+1);

    if (buf) {

    StringCchCopy(buf,len1+len2,s1);

    StringCchCat(buf,len1+len2,s2);

    }

    // do other stuff with buf

    if (buf) free(buf);

    return true;

    }

    "

    I am continuosly failing to see a statement which could trigger a crash.

    The statements which could cause a crash if the value of len2 is 0xFFFFFFFF and value of len1 is 64,are potentially:

    1. StringCchCat(buf,len1+len2,s2);

    2.if (buf) free(buf);

    The reasoning which my silly mind tries to give me is

    a.Statement 1 wont crash since len2+len2 gives 63 which is well within the buffer length we have allocated for buf.

    b.Statement 2 will crash only when buffer s1 is uninitialized.

    c.It is not possible to allocate 0xFFFFFFFF bytes to a buffer.

    I tried different combinations but I m not able to make the code crash.

    Could you please enlighten me,if possible and if you have a moment?

    Thank you very much for your time.

  2. c says:

    Testing to see if these are turned on yet :)

  3. Alan says:

    Wow, that sucks really amazingly!

    So using the nice SafeInt class I could express the concept of safely evaluating a*b+1 by writing, well, a*b+1, but you think it would be much better to write

    if (SUCCEEDED(IntToDWord(cElem,&cbElem)) &&

    SUCCEEDED(DWordMult(cbBlockSize, cbElem, &cbElem)) &&

    SUCCEEDED(DWordAdd(cbElem,1,&cbElem)))

    I’m truly appalled. Making code that much less readable makes it harder to see what it’s doing, which is going to cause bugs (both security related and non-security related).

    Not to mention having to check return values on *every single arithmetic operation* instead of getting a sensible exception and handling it in one place.

    What were you thinking? Are you doing this for some kind of bet or joke?

  4. Karridine says:

    Michael, your name came up when I Googled "QuickTime blogs", and I went down-page 15 posts but couldn’t find a more appropriate thread, so I share with you (and your VAST reading audience) that I’ve downloaded and installed the eTunes/QuickTime bundle

    BUT!

    Whenever I click on a QT-enabled website, it STILL tells me to upgrade to QT 4.5 or 7.5 or whatever it is! And I’ve RE-installed QT twice last week, for a total of 3 installations on WinXP platform, and STILL the detour when I hit a QT-webFunction…

    And Yahoo’s YahooSetupVideoPlayer.exe is infected with the Trojan Spy.Bombka so I cannot install it… I can’t defuse it, excise the bad bits, or notify Yahoo to deal with this PROBLEM, durn it! I still can’t play "Its In the Koran!"

  5. David LeBlanc says:

    There’s a few things to remember about taking this approach – the first is that conversion between integer types on entry into a function will happen without any warning, IIRC, even at warning level 4. So if you do have signed values coming in and forget to use the conversion function, you’ll get unexpected results and no warning. Another aspect of this is if you don’t know what casting behavior to expect – what if the input is a short? If I pass a short to a function expecting an unsigned int, serious mayhem can ensue since it will sign extend first.

    The second thing is that there are some common cases where you’ll get a lot better perf by writing a dedicated function. Specifically, a*b+c is a common case where if you have 32-bit inputs, you can upcast all of them to either _int64 or unsigned _int64 (depending on the sign of the inputs), and if they’re all the same sign, the operations cannot overflow internally, and you can then check for overflow on the whole value instead of having many chained comparisons. For this specific case, you’d get better perf than SafeInt as well. We wrote such a function for Office and used it fairly often.

    Lastly, while reviewing Office’s MSO.dll for integer problems, we found a number of cases where someone changed from signed to unsigned, and created vulnerable code that wasn’t previously a problem. While it is true that unsigned numbers are a LOT easier to check, if you had something along the lines of if(input < 0) return InputError();, then changed input from signed to unsigned and forgot to correctly change all the code (to something like if(input > max_size) ), then you’re creating problems.

    The thing to remember is that when correcting integer problems, you really have to be careful. It’s a complex problem.

    While I am indeed biased, I have also spent a great deal of time thinking about this problem. If you are using C++, SafeInt is much less likely to allow errors than any C constructs. As an example, I was shown an early version of IntSafe and a code snippet that ‘proved’ IntSafe was substantially better performing than SafeInt. It turned out that SafeInt was checking 3 operations, and IntSafe was checking one. It wasn’t a fair comparison, and more importantly, SafeInt was giving the programmer a LOT more protection. If you’re stuck with C, then you obviously cannot use SafeInt, and in that case, IntSafe can be a big help. Nonetheless, be aware of the trade-offs and where you’ll need to be doing more work to avoid mistakes.

  6. I was asked last week for a list of &quot;drop-in-and-more-secure&quot; replacements, created at Microsoft,&amp;nbsp;for…

  7. HoraceWang says:

    List of useful security libraries