Re: Fwd: [PATCH] Fix Heap-use-after-free in hash_table_destroy

[Derek Martin]

On Thu, Mar 27, 2025 at 04:51:46PM +0530, shivam tiwari wrote:
> Dear Wget Developers,
> 
> This patch fixes a Heap-use-after-free vulnerability in the
> hash_table_destroy function.

Note:

    diff --git a/src/hash.c b/src/hash.c
    index 670ec1cf..00e07eef 100644
    --- a/src/hash.c
    +++ b/src/hash.c
    @@ -305,8 +305,13 @@ hash_table_new (int items,
     void
     hash_table_destroy (struct hash_table *ht)
     {
    -  xfree (ht->cells);
    -  xfree (ht);
    +  if (ht)
    +  {
    +    xfree (ht->cells);
    +    ht->cells = NULL;
    +    xfree (ht);
    +    ht = NULL;
    +  }
     }

The line:

    ht = NULL;

is ineffectual.  In this function, ht is a local (function scope) copy
of the pointer.  For the intended thing to happen, you need this:

void
hash_table_destroy(struct hash_table **ht)
{
    if (ht && *ht)
    {
        xfree(*ht->cells);
        *ht->cells = NULL;
        xfree(*ht);
        *ht = NULL;
    }
}

And then everywhere it's called, you need to change:

    struct hash_table *foo;
    ...
    hash_table_destroy(foo);
    
to:

    hash_table_destroy(&foo);

Alternatively, you could remove the assignment, and use a macro
similar to:

#define hash_table_free(htp) do { hash_table destroy(htp); htp = NULL; } while 
(0);

(The solution with the least code changes would be to rename the
function and give the macro its original name...)


And for the sake of completeness, a short example to demonstrate this:

$ cat free.c
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

struct hash_table {
    void *cells;
};

void hash_table_destroy(struct hash_table *ht)
{
    if (ht)
    {
        free(ht->cells);
        /* don't need to check ht->cells, free(NULL) is a no-op on all 
non-broken systems */
        ht->cells = NULL;
        free(ht);
        ht = NULL;
    }
}

int main(int argc, char **argv)
{
    struct hash_table *foo;
    foo = (struct hash_table *)malloc(sizeof *foo);
    foo->cells = NULL;
    printf("Address of *foo is %p\n", foo);
    hash_table_destroy(foo);
    printf("Address of *foo is %p\n", foo);
    return 0;
}

$ gcc -o free free.c
$ ./free
Address of *foo is 0x6243cfb262a0
Address of *foo is 0x6243cfb262a0