This is the mail archive of the mailing list for the glibc project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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

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
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.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]