Alright all, here is the next bug. This one is courtesy of Mike Howard.

__declspec(noinline) void* AllocBlocks(size_t cBlocks) {

      // allocating no blocks is an error
      if (cBlocks == 0)
            return NULL; 

      // Allocate enough memory
      // Upcast the result to a 64-bit integer
      // and then check result against 32-bit UINT_MAX 
      // this makes sure there's no integer overflow

      ULONGLONG alloc = cBlocks * 16;
      return (alloc < UINT_MAX)

      ? malloc(cBlocks * 16)
      : NULL;

}

Solution

We did not have a lot of activity around this bug. This one was tough.

The problem is that the int overflow check doesn’t work…

__declspec(noinline) void* AllocBlocks(size_t cBlocks) {

      // allocating no blocks is an error
      if (cBlocks == 0)
            return NULL;

      // Allocate enough memory
      // Upcast the result to a 64-bit integer
      // and then check result against 32-bit UINT_MAX 
      // this makes sure there's no integer overflow

      ULONGLONG alloc = cBlocks * 16;
      return (alloc < UINT_MAX)

      ? malloc(cBlocks * 16)
      : NULL;

}

These lines are all wrong

      ULONGLONG alloc = cBlocks * 16;
      return (alloc < UINT_MAX)

cBlocks * 16 is a 32-bit multiply, so the multiply always yields a 32-bit value. Then that result is assigned to a ULONGLONG, but the result is always less than UINT_MAX, because the calculation may have already overflowed. Fix is:

      ULONGLONG alloc = (ULONGLONG)cBlocks * 16;