diff mbox

malloc: harden removal from unsorted list

Message ID 9b74b50e-66a6-3321-5668-d89b35a21cd8@google.com
State Superseded, archived
Headers

Commit Message

Francois Goichon Feb. 23, 2018, 12:17 p.m. UTC 
  There isn't any linked list check when getting chunks out of the 
unsorted list which allows an attacker to write &av->top 
(unsorted_chunks chunk) almost anywhere (e.g. into a bin of their 
choosing to get it out in a subsequent allocation), with a relatively 
simple corruption:

--
   // Initial state
   long * c = malloc(0x100);
   malloc(0x100);
   free(c);

   // Corruption: point c->bk to bin used in alloc after next
   c[1] += (0x110 >> 4)*8*2 - 8;
   malloc(0x100);
   c = malloc(0x100);  // &av->top
--

Controlling bins is likely to be enough to gain code execution by 
overwriting farther ahead in .data or .bss (e.g. __free_hook).

Tested on i386 and x86_64.


	* malloc/malloc.c (_int_malloc): Added check before removing from
	unsorted list.

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

Comments

Florian Weimer Feb. 25, 2018, 3:15 p.m. UTC | #1 
* 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.
diff mbox

Patch

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;