Integer Overflow and operator::new


As Raymond Chen pointed out last year (http://blogs.msdn.com/oldnewthing/archive/2004/01/29/64389.aspx), there is a potential integer overflow when calling operator::new. The C++ compiler in Visual Studio 2005 automatically generates defensive code to mitigate this potential vulnerability.


 


Code like this:


 


class CFoo {


public:


      CFoo() {m_p = NULL;}


private:


      void *m_p;


};


 


void *func(size_t count) {


      if (!count) return NULL;


      return new CFoo[count];


}


 


When compiled with VS2005, yields output like this:


 


; _count$ = esi


 


; 14   :    if (!count) return NULL;


 


  00000     85 f6       test esi, esi


  00002     74 36       je   SHORT $LN4@func


 


; 15   :    return new CFoo[count];


 


  00004     33 c9       xor  ecx, ecx


  00006     8b c6       mov  eax, esi


  00008     ba 04 00 00 00    mov  edx, 4


  0000d     f7 e2       mul  edx


  0000f     0f 90 c1    seto cl


  00012     f7 d9       neg  ecx


  00014     0b c8       or   ecx, eax


  00016     51          push ecx


  00017     e8 00 00 00 00    call ??2@YAPAXI@Z     ; operator new


 


As you can see (assuming you can read assembly code!) the multiply occurs (mul edx) and then the CL register is set or not set depending on the value if the overflow flag. Now here’s the funky part, ECX will end up being 0x00000000 or 0xFFFFFFFF, and because of the next operation (or ecx) the ECX register will either be 0xFFFFFFFF or the value held in EAX, which is result of the initial multiply. This is then passed to operator::new which will fail in the face of 2^N-1 by returning NULL or throwing an exception.


 


It’s better to fail, and potentially crash the application, than have an integer overflow that may lead to code execution.


 


Of course, the correct solution is get the code right in the first place by performing good arithmetic hygiene, and in this case, restrict the maximum number of objects you’ll create:


 


void *func(size_t count) {


      if (!count || count > MAX_FOO_ALLOWED) return NULL;


      return new CFoo[count];


}


 

Comments (7)

  1. Alexei Zakharov says:

    Is there any rational reason to use plain arrays in today’s C++ code? Even if there wasn’t any possibility of integer overflow there are so many factors against C++ arrays (like type safety, exception safety to name a few) and so many proper abstraction classes developed to substitute arrays in C++ that there is no excuse to use plain vanilla C++ arrays in your code anymore. The simplest suitable general purpose substitute is std::vector. It’s been a part of the standard for 7 years. It’s just a shame not to know about it and not to advocate its use instead of arrays. It’s got neither problems listed above nor the integer overflow problem. The bottom line is you can enhance the compiler to protect you from the unsafe C++ constructs (C legacy really), but at the end of the day they are still inherently unsafe and dangerous and your code is still vulnerable. It’s just not the way to go with this language. Better, safer, higher level abstractions is the way to go. The language provides you with the tools you need to build them, and it also provides an excellent standard library that already has many abstractions you need.

  2. mTIE says:

    So why not put an overflow check inside the implementation of new[]()??

    I would argue that if new CFoo[(1<<31)+1] runs a pointer to an array of [1] allocated object, that’s a bug in new, and should be fixed in the RTL.

  3. Peter says:

    The mul instruction is the slowest way to multiply the number by 4. You can get the same result faster with LEA edx, [edx * 4] or with SHL edx, 2 or with ADD edx, edx / ADD edx, edx

    What was optimization switches (/O1, /O2) when you compiled this code?

  4. Albatross says:

    The new operation compiler "last chance" security protection is OK.

    However, I think a clarification on the usage of the MAX_FOO_ALLOWED upper layer protection proposed is necessary.

    The nature of the "integer overflow" scenario is because of the implicit sizeof multiplication that the new operator will execute, for such reason MAX_FOO_ALLOWED must be calculated based on that.

    #define MAX_CPUREG_VALUE (2^CPUBITS)

    #define MAX_FOO_ALLOWED

    (MAX_CPUREG_VALUE / sizeof(CFoo))

  5. Albatross says:

    The new operation compiler "last chance" security protection is OK.

    However, I think a clarification on the usage of the MAX_FOO_ALLOWED upper layer protection proposed is necessary.

    The nature of the "integer overflow" scenario is because of the implicit sizeof multiplication that the new operator will execute, for such reason MAX_FOO_ALLOWED must be calculated based on that.

    #define MAX_CPUREG_VALUE (2^CPUBITS)

    #define MAX_FOO_ALLOWED

    (MAX_CPUREG_VALUE / sizeof(CFoo))

  6. MS07-004 does not affect Windows Vista, even though the coding bug is there. Why? The bug is an integer

Skip to main content