On Fri, Oct 30, 2015 at 11:51:17PM -0400, Daniel Micay wrote:
> On 26/10/15 04:19 PM, Daniel Micay wrote:
> > This is an improved revision of my earlier patch.
> >
> > It now validates the junk data in the delayed_chunks array in an atexit
> > handler
> > too, rather than just when allocations are swapped out.
> >
> > It will now catch this simple UAF 100% of the time:
> >
> > #include <stdlib.h>
> > #include <stdio.h>
> >
> > int main(void) {
> > size_t i;
> > char *p;
> > for (i = 0; i < 32; i++) {
> > p = malloc(16);
> > if (!p) return 1;
> > }
> >
> > p = malloc(16);
> > if (!p) return 1;
> > free(p);
> > *p = 5;
> >
> > for (i = 0; i < 4; i++) {
> > p = malloc(16);
> > if (!p) return 1;
> > free(p);
> > }
> > return 0;
> > }
> >
> > In general, it depends on the allocation still being in the delayed chunks
> > array when the use-after-free happens. This means a larger delayed chunks
> > array would improve the detection rate.
>
> That revision was missing a NULL check as it got lost when refactoring
> and the test cases weren't short enough to trigger it. Properly working
> implementation:
Hi Daniel,
I did not have time to look into this up until now and I'm pretty sure
I will not have much time in the near future. But after a casual look
I consider this a very nice addition, so please do continue to persue
this.
To all others: please test and give Daniel feedback,
-Otto
>
> diff --git a/stdlib/malloc.c b/stdlib/malloc.c
> index 424dd77..c408594 100644
> --- a/stdlib/malloc.c
> +++ b/stdlib/malloc.c
> @@ -182,6 +182,7 @@ struct malloc_readonly {
> int malloc_freeunmap; /* mprotect free pages PROT_NONE? */
> int malloc_hint; /* call madvice on free pages? */
> int malloc_junk; /* junk fill? */
> + int malloc_validate; /* validate junk */
> int malloc_move; /* move allocations to end of page? */
> int malloc_realloc; /* always realloc? */
> int malloc_xmalloc; /* xmalloc behaviour? */
> @@ -218,6 +219,8 @@ static void malloc_exit(void);
> #define CALLER NULL
> #endif
>
> +static void validate_delayed_chunks(void);
> +
> /* low bits of r->p determine size: 0 means >= page size and p->size
> holding
> * real size, otherwise r->size is a shift count, or 1 for malloc(0)
> */
> @@ -560,6 +563,12 @@ omalloc_init(struct dir_info **dp)
> case 'J':
> mopts.malloc_junk = 2;
> break;
> + case 'v':
> + mopts.malloc_validate = 0;
> + break;
> + case 'V':
> + mopts.malloc_validate = 1;
> + break;
> case 'n':
> case 'N':
> break;
> @@ -608,6 +617,9 @@ omalloc_init(struct dir_info **dp)
> }
> }
>
> + if (!mopts.malloc_junk)
> + mopts.malloc_validate = 0;
> +
> #ifdef MALLOC_STATS
> if (mopts.malloc_stats && (atexit(malloc_exit) == -1)) {
> static const char q[] = "malloc() warning: atexit(2) failed."
> @@ -616,6 +628,12 @@ omalloc_init(struct dir_info **dp)
> }
> #endif /* MALLOC_STATS */
>
> + if (mopts.malloc_validate && (atexit(validate_delayed_chunks) == -1)) {
> + static const char q[] = "malloc() warning: atexit(2) failed."
> + " Will not be able to check for use after free\n";
> + write(STDERR_FILENO, q, sizeof(q) - 1);
> + }
> +
> while ((mopts.malloc_canary = arc4random()) == 0)
> ;
>
> @@ -1190,6 +1208,35 @@ malloc(size_t size)
> /*DEF_STRONG(malloc);*/
>
> static void
> +validate_junk(void *p) {
> + struct region_info *r;
> + struct dir_info *pool = getpool();
> + size_t byte, sz;
> + if (p == NULL)
> + return;
> + r = find(pool, p);
> + if (r == NULL)
> + wrterror("bogus pointer in validate_junk", p);
> + REALSIZE(sz, r);
> + for (byte = 0; byte < sz; byte++) {
> + if (((char *)p)[byte] != SOME_FREEJUNK) {
> + wrterror("use after free", p);
> + return;
> + }
> + }
> +}
> +
> +static void
> +validate_delayed_chunks(void) {
> + struct dir_info *pool = getpool();
> + int i;
> + if (pool == NULL)
> + return;
> + for (i = 0; i < MALLOC_DELAYED_CHUNK_MASK + 1; i++)
> + validate_junk(pool->delayed_chunks[i]);
> +}
> +
> +static void
> ofree(void *p)
> {
> struct dir_info *pool = getpool();
> @@ -1253,6 +1300,8 @@ ofree(void *p)
> wrterror("double free", p);
> return;
> }
> + if (mopts.malloc_validate)
> + validate_junk(p);
> pool->delayed_chunks[i] = tmp;
> }
> if (p != NULL) {
