Jump to navigation

Poupou's Corner of the Web

Looking for perfect security? Try a wireless brick.
Otherwise you may find some unperfect stuff here...


Watch out for overflows

I've been wanting to write this entry for about one year now. Not quite sure why it didn't occur before (but lazyness would be my prime suspect ;-).

Using .NET makes it hard, but not impossible, for buffer overflows to occurs. However other overflows, like integer overflows, exists and are still very real in a managed environment (and can even lead to buffer overflows).

Example #1:

// Note: I used unsafe to keep the example simple but the same problem // would exists if we were calling an internal call or p/invoking. public unsafe void Zeroize (byte[] buffer, int startIndex, int count) { int i = 0; fixed (byte* ptr = &buffer[startIndex]) { byte* p = ptr; while (i++ < count) *p++ = 0x00; } }

Now it's easy to imagine a lot of nasty problem with this code as none of the parameters are validated before being used. It's even worst as we have declared this method as public - so we can't assume the caller knows "how to call it". Anyway we should never assume that so let's check them...

Example #2:

public unsafe void Zeroize (byte[] buffer, int startIndex, int count) { if (buffer == null) throw new ArgumentNullException ("buffer"); if (startIndex < 0) throw new ArgumentOutOfRangeException ("negative", "startIndex"); if (count < 0) throw new ArgumentOutOfRangeException ("negative", "count"); if (startIndex + count > buffer.length) throw new ArgumentOutOfRangeException ("startIndex + count > this.length"); int i = 0; fixed (byte* ptr = &buffer[startIndex]) { byte* p = ptr; while (i++ < count) *p++ = 0x00; } }

This second version is better because it adds validation of all parameters. However it doesn't validate all of them correctly. For example what if startIndex or count is big, very big like Int32.MaxValue...

byte[] array = new byte[16]; Zeroize (array, 15, Int32.MaxValue);

In this case we have:

  • buffer is non null, first check is ok;
  • startIndex == 15, which is greater than 0, second check is ok;
  • count == Int32.MaxValue, which is also greater than 0, third check is ok;
  • startIndex + count = 15 + Int32.MaxValue == -2147483634 which is smaller than 16, fourth check is ok :(

All checks are positive so the unsafe code can execute, problem is (i++ < count) won't happen anytime soon - at least not before a lot of data gets zeroized. It this common ?

You bet! The .NET documentation often (or maybe always ?) documents the check as an addition (or doesn't even document the check) but the framework itself isn't coded like that. I have yet to found one integer overflow in MS implementation of the framework but I did found other interesting stuff, which were all reported, so I guess they have some automated tool to check for this (it's too good to be true ;-).

Integer overflow are usually much easier to fix than to find (even with a common pattern). In this case the problem can be fixed by reversing the operation, i.e. doing a substraction instead of an addition.

// re-ordered to avoid possible integer overflow // (experience shows that it helps to comment this stuff) if (startIndex > this.length - count) throw new ArgumentOutOfRangeException ("startIndex + count > this.length");

We could have an integer underflow if we were only using this check (e.g. if startIndex or count were equals to Int32.MinValue). This is why there are (or there should be) checks for both the startIndex and count parameters to ensure they can't be negative.

This isn't the only case where integer [over|under]flow can bite you. In many case the VM will protect you against this problem (e.g. if we haven't used unsafe code). However this can be very dangerous if your code, like it's often the case inside mscorlib.dll, System.dll and in all interop assemblies, use those values to deal with unmanaged ressources (e.g. memory).

6/10/2005 15:19:27 | Comments

The views expressed on this website/weblog are mine alone and do not necessarily reflect the views of my employer.