Re: [PATCH] malloc/malloc.c: Mitigate null-byte overflow attacks

[Moritz Eckert <m dot eckert at cs dot ucsb dot edu>]

On 10/23/2017 01:41 PM, DJ Delorie wrote:

> I'm OK with the patch in theory, but...
>
> unlink() is called from seven places; you have patched two.  Are the
> other five open to this bug?  Perhaps it would be better to add another
> parameter to the unlink() macro to centralize this check and enforce it
> everywhere?
I thought about that, but unlink is used in a forward and backward way.
Therefore, adding another parameter and use it to centralize this check
should be possible, however it requires the caller to know which sizes 
have to be
passed for comparison. It seemed like this could cause some confusion. 
The current code is a great
example that given the wrong sizes to compare, the check gets 
non-functional.

I only placed the check before backward unlinks, since shrinking the 
size with a null-byte-overflow
will always lead to a forward unlink to controlled memory, which is not 
preventable by any such check.

> I wonder if we should add a "size_is_sane()" macro to check for
> unreasonable sizes before we use them to compute pointers.
That sounds like a good idea to me. Would you prefer a separate macro
for prev_size and size that only gets the current chunk as a parameter or
a single macro that gets a parameter what to check for?
> Also, your mailer is corrupting your patch; I had to apply it by hand to
> review it.  It's wrapping lines and using 0xa0 spaces instead of 0x20.
> Attaching it as inline-text might help, instead of just pasting it in to
> the body.
That's unfortunate, I'm sorry. Will double check next time.