On 9/16/20 9:23 PM, Jeff Law wrote:
On 9/15/20 1:47 PM, Martin Sebor wrote:
Overflowing the size of a dynamic allocation (e.g., malloc or VLA)
can lead to a subsequent buffer overflow corrupting the heap or
stack. The attached patch diagnoses a subset of these cases where
the overflow/wraparound is still detectable.
Besides regtesting GCC on x86_64-linux I also verified the warning
doesn't introduce any false positives into Glibc or Binutils/GDB
builds on the same target.
Martin
gcc-96838.diff
PR middle-end/96838 - missing warning on integer overflow in calls to
allocation functions
gcc/ChangeLog:
PR middle-end/96838
* calls.c (eval_size_vflow): New function.
(get_size_range): Call it. Add argument.
(maybe_warn_alloc_args_overflow): Diagnose overflow/wraparound.
* calls.h (get_size_range): Add argument.
gcc/testsuite/ChangeLog:
PR middle-end/96838
* gcc.dg/Walloc-size-larger-than-19.c: New test.
* gcc.dg/Walloc-size-larger-than-20.c: New test.
If an attacker can control an integer overflow that feeds an allocation,
then they can do all kinds of bad things. In fact, when my son was
asking me attack vectors, this is one I said I'd look at if I were a bad
guy.
I'm a bit surprised you can't just query the range of the argument and
get the overflow status out of that range, but I don't see that in the
APIs. How painful would it be to make that part of the API? The
conceptual model would be to just ask for the range of the argument to
malloc which would include the range and a status bit indicating the
computation might have overflowed.
I agree that being able to evaluate an expression in an as-if
infinite precision (in addition to its type) would be helpful.
I mentioned it to Aldy in response to the irange best practices
document. He says there's no way to do that and no plans to
make it possible.
But just to make sure I understood correctly, let me ask again
using an example:
void* f (size_t n)
{
if (n < PTRDIFF_MAX / 2)
n = PTRDIFF_MAX / 2;
return malloc (n * sizeof (int));
}
Can the unsigned wraparound in the argument be readily detected?
On trunk, this ends up with the following:
# RANGE [4611686018427387903, 18446744073709551615]
_6 = MAX_EXPR <n_2(D), 4611686018427387903>;
# RANGE [0, 18446744073709551615] NONZERO 18446744073709551612
_1 = _6 * 4;
...
p_5 = mallocD.1206 (_1); [tail call]
...
return p_5;
so _1's range reflects the wraparound in size_t, but _6's range
has enough information to uncover it. So detecting it is possible
and is done in the patch so we get a warning:
warning: argument 1 range [18446744073709551612, 0x3fffffffffffffffc] is
too large to represent in ‘long unsigned int’ [-Walloc-size-larger-than=]
6 | return malloc (n * sizeof (int));
| ^~~~~~~~~~~~~~~~~~~~~~~~~
The code is very simplistic and only handles a small subset of cases.
It could be generalized and exposed by a more generic API but it does
seem like the ranger must already have all the logic built into it so
if it isn't exposed now it should be a matter of opening it up.
Martin
