When reallocating, if we allocate new space, we need to copy the old
allocation's bytes into the new space. There are old_size number of bytes in
the old allocation, but we were accidentally copying new_size number of bytes,
which could lead to copying bytes into the realloc'd space from past the chunk
that we're bump allocating out of, from unknown memory.
If an attacker can cause reallocs, and can read the realoced data back, this
could allow them to read things from other regions of memory that they shouldn't
be able to. For example, if some crypto keys happened to live in memory right
after a chunk we were bump allocating out of, this could allow the attacker to
read the crypto keys.
Beyond just fixing the bug and adding a regression test, I've also taken two
additional steps:
While we were already running the testsuite under valgrind in CI, because
valgrind exits with the same code that the program did, if there are invalid
reads/writes that happen not to trigger a segfault, the program can still exit
OK and we will be none the wiser. I've enabled the --error-exitcode=1 flag for
valgrind in CI so that tests eagerly fail in these scenarios.
I've written a quickcheck test to exercise realloc. Without the bug fix in
this patch, this quickcheck immediately triggers invalid reads when run under
valgrind. We didn't previously have quickchecks that exercised realloc
beacuse realloc isn't publicly exposed directly, and instead can only be
indirectly called. This new quickcheck test exercises realloc via
bumpalo::collections::Vec::resize and
bumpalo::collections::Vec::shrink_to_fit calls.
When reallocating, if we allocate new space, we need to copy the old allocation's bytes into the new space. There are
old_size
number of bytes in the old allocation, but we were accidentally copyingnew_size
number of bytes, which could lead to copying bytes into the realloc'd space from past the chunk that we're bump allocating out of, from unknown memory.If an attacker can cause
realloc
s, and can read therealoc
ed data back, this could allow them to read things from other regions of memory that they shouldn't be able to. For example, if some crypto keys happened to live in memory right after a chunk we were bump allocating out of, this could allow the attacker to read the crypto keys.Beyond just fixing the bug and adding a regression test, I've also taken two additional steps:
While we were already running the testsuite under
valgrind
in CI, becausevalgrind
exits with the same code that the program did, if there are invalid reads/writes that happen not to trigger a segfault, the program can still exit OK and we will be none the wiser. I've enabled the--error-exitcode=1
flag forvalgrind
in CI so that tests eagerly fail in these scenarios.I've written a quickcheck test to exercise
realloc
. Without the bug fix in this patch, this quickcheck immediately triggers invalid reads when run undervalgrind
. We didn't previously have quickchecks that exercisedrealloc
beacuserealloc
isn't publicly exposed directly, and instead can only be indirectly called. This new quickcheck test exercisesrealloc
viabumpalo::collections::Vec::resize
andbumpalo::collections::Vec::shrink_to_fit
calls.Fixes #69