Integer overflow in operator new
Hi all, apologies if this has been discussed before, but I
couldn't find anything about this issue in gcc mailing list
archives.
Use of operator new (et al) appears to have an integer overflow;
this function:
int * allocate_int(size_t n)
{
return new int[n];
}
with gcc-4.1 on IA-32 compiles to:
_Z12allocate_intj:
pushl %ebp
movl%esp, %ebp
subl$8, %esp
movl8(%ebp), %eax
sall$2, %eax
movl%eax, (%esp)
call_Znaj
leave
ret
which is equivalent to the compilation of:
int * allocate_int(size_t n)
{
return (int*) operator new[](4 * n);
}
"4 * n", unchecked, is vulnerable to integer overflow. On IA-32,
"new int[0x4001]" becomes equivalent to "new int[1]". I've
verified this on gcc-2.95 through 4.1. For larger objects the
effects are exaggerated; smaller counts are needed to overflow.
This is similar to the calloc integer overflow vulnerability in
glibc, which was fixed back in 2002. Interestingly, RUS-CERT
2002-08:02 did mention 'operator new', and so did Bugtraq 5398.
http://cert.uni-stuttgart.de/advisories/calloc.php
http://www.securityfocus.com/bid/5398/discuss
See also this 2004 article by Raymond Chen:
http://blogs.msdn.com/oldnewthing/archive/2004/01/29/64389.aspx
Possible fixes might be to abort or to pass ULONG_MAX (0x)
to 'operator new' and let it return NULL or throw bad_alloc.
At least one other compiler already specifically guards against
integer overflow in 'operator new'.
--
Karl 2007-04-06 07:30
Re: Integer overflow in operator new
> On 2007-04-06 15:35 PDT, J C Pizarro writes:
J> A possible workaround could be it but it's vulnerable if
J> it's defined with -DNDEBUG :
J> int * allocate_int(size_t n) {
J> // it's another integer overflow, a positive can
J> // become to a negative.
J> //n=1073741823 (0x3FFF) => n*4=-4
J> //(0xFFFC) return (int*) operator
J> //new[](-4); !!! it's easy for
J> buffer overflow.
J> assert(0 <= (4 * n));
J> // it's an assert against your integer overflow.
J> assert((4ULL * n) <= ULONG_MAX); return (int*)
J> operator new[](4 * n);
J> }
Good points.
Regarding negatives, I believe 'operator new' takes a size_t,
which is unsigned, but if it were signed it, the multiplication
would indeed be in danger of creating a negative.
If possible, I would prefer a solution that's built-in to operator
new. I was thinking it should be implemented when code is
generated, for example using jc/jo/seto on i386.
--
Karl 2007-04-06 15:41
Re: Integer overflow in operator new
> On 2007-04-06 16:12 PDT, Lawrence Crowl writes: Lawrence> Asking programmers to write extra code for rare Lawrence> events, has not been very successful. Well put Lawrence, I agree; I didn't expect strong opposition. I doubt we'd find much code in the wild that checks for integer overflow before every 'new'. I'd like to reiterate that the equivalent calloc issue was treated as a serious vulnerability and fixed right away. >From the programmer's point of view, the multiplication isn't obvious. For all they know, it's a 'for' loop that iterates per element, allocating (sizeof object) bytes in each iteration. So it's not obvious why a check would even be necessary to a typical user. I would also note that all non-char uses of operator new that I looked at in gcc's C++ headers are lacking integer overflow checks. Lawrence> It would be better if the compiler incorporated this Lawrence> check into operator new, though throwing an Lawrence> exception rather than asserting. The compiler Lawrence> should be able to eliminate many of the Lawrence> conditionals. Indeed, recent research has found that even checking for overflow on *every arithmetic operation* is only a 5% run-time overhead. A default check on only 'operator new' invocations is quite cheap - a single jump-if-overflow instruction that fares well with branch prediction. Handling the overflow case can be moved faraway so it doesn't affect instruction cache performance. Fixing it in user code at call sites is cumbersome - there's no way to wrap a macro around 'operator new', no way to do it in custom 'operator new' since the implicit multiplication has already occurred by then. It would have to be an explicitly typed check at every use of 'operator new'. -- Karl 2007-04-06 18:15
