malloc: harden removal from unsorted list

Message ID c7071e51-7545-c618-85dc-2f10f3b88e4d@google.com
State Committed, archived
Headers

Commit Message

Francois Goichon Feb. 26, 2018, 10:48 a.m. UTC
  On the exploitability of this, the += was just there to explicitly point
out the targeted bin, but an actual exploitation would more likely look
at  overwriting the least significant byte(s), or the full address if in
a position to leak a free or uninitialized chunk beforehand.

I believe this is in line with other attacks that malloc tries to
prevent, i.e. where controlling a single pointer is sufficient to get
significant control over the application's behavior. I agree it's a fine
line though and that it's not malloc's role to stop exploitation of edge
cases where the attacker has an unreasonable amount of control.

Happy to try and put numbers on the performance impact of this change
if it's a concern here - if yes is there a benchmark that would be best
suited to this usecase?

Rolled back the existing messages as suggested:


---
  malloc/malloc.c | 2 ++
  1 file changed, 2 insertions(+)
  

Patch

diff --git a/malloc/malloc.c b/malloc/malloc.c
index 58f9acd4d1..fd1a263e9e 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -3775,6 +3775,8 @@  _int_malloc (mstate av, size_t bytes)
              }

            /* remove from unsorted list */
+          if (__glibc_unlikely (bck->fd != victim))
+            malloc_printerr ("malloc(): corrupted unsorted chunks 3");
            unsorted_chunks (av)->bk = bck;
            bck->fd = unsorted_chunks (av);