fastbin_dup_consolidate wrong explanation

[k4lizen]

Allocating a tcache-sized chunk will trigger the malloc_consolidate and merge [...]

It doesn't need to be tcache-sized. Changing the chunk size of p3 and p4 to a large-bin works perfectly. This is because this malloc_consolidate is hit: https://elixir.bootlin.com/glibc/latest/source/malloc/malloc.c#L4041

Even if tcache was disabled, and it was a small chunk, I'm pretty sure this: https://elixir.bootlin.com/glibc/latest/source/malloc/malloc.c#L4468 would still be hit (and it is what is being hit in the exploit currently).

Even the further text:

        printf("The double free added the chunk referenced by p1 \n");
    printf("to the tcache thus the next similar-size malloc will\n");
    printf("point to p3: p3=%p, p4=%p\n\n",p3, p4);

Is a bit misleading, since the exploit works if p3 gets consolidated with top and doesn't go into the tcache (which is the case when larger malloc values for p3 and p4 are provided).

As a side note:

    void* p1 = calloc(1,0x40);

    printf("Allocate another chunk of the same size p1=%p \n", p1);
    printf("Freeing p1 will add this chunk to the fastbin list...\n\n");
    free(p1);

Utilizes a non-obvious mechanic of calloc to not take p1 out of the tcache. It would probably be better to allocate p1 with malloc before the tcache is freed, since its more obvious what is going on.

[k4lizen]

I can write a PR for cleaning up this file if given the go-ahead

[Kyle-Kyle]

I think the point is malloc_consolidate can allow the same chunk to be taken twice. And the idea of the existing description is to teach readers that freeing tache-sized chunk can also trigger malloc_consolidate. unsortedbin-sized chunk can trigger it is a widely known behavior so the technique forgets to mention that as well. It'd be definitely more complete to mention it.

Notice that when doing free, there is only tcache and unsortedbin (we leave out mmap here). smallbin and largebin are basically sorted unsortedbins.

About the calloc call, I'd say it'd be better if we keep its current implementation but add some explanations for it. As a repo with educational purposes, it should guide readers to try out and understand different new mechanisms.

[Kyle-Kyle]

And please feel free to clean up the file and PRs are welcome

[k4lizen]

I am taking a look at this right now and wanted to mention that I was wrong:

Even if tcache was disabled, and it was a small chunk, I'm pretty sure this: https://elixir.bootlin.com/glibc/latest/source/malloc/malloc.c#L4468 would still be hit (and it is what is being hit in the exploit currently).

Currently in the exploit this is in-fact being hit. The other malloc_consolidate thats present in _int_malloc is really hard to hit since its only reached when the top chunk is too small for the allocation.