Message ID | 9b74b50e-66a6-3321-5668-d89b35a21cd8@google.com |
---|---|
State | New |
Headers | show |
Series | malloc: harden removal from unsorted list | expand |
* Francois Goichon: > - malloc_printerr ("malloc(): corrupted unsorted chunks"); > + malloc_printerr ("malloc(): corrupted unsorted chunks 2"); I suggest not to change the existing messages, so that we can interpret them without closely looking at the glibc version. I think that's more useful than keeping the messages in source code order.
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(+) 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);
diff --git a/malloc/malloc.c b/malloc/malloc.c index 58f9acd4d1..17d373eca9 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"); unsorted_chunks (av)->bk = bck; bck->fd = unsorted_chunks (av); @@ -3941,7 +3943,7 @@ _int_malloc (mstate av, size_t bytes) bck = unsorted_chunks (av); fwd = bck->fd; if (__glibc_unlikely (fwd->bk != bck)) - malloc_printerr ("malloc(): corrupted unsorted chunks"); + malloc_printerr ("malloc(): corrupted unsorted chunks 2"); remainder->bk = bck; remainder->fd = fwd; bck->fd = remainder; @@ -4045,7 +4047,7 @@ _int_malloc (mstate av, size_t bytes) bck = unsorted_chunks (av); fwd = bck->fd; if (__glibc_unlikely (fwd->bk != bck)) - malloc_printerr ("malloc(): corrupted unsorted chunks 2"); + malloc_printerr ("malloc(): corrupted unsorted chunks 3"); remainder->bk = bck; remainder->fd = fwd; bck->fd = remainder;