Hi, `xfree` is a macro that already NULLifies the pointer after calling free().So your patch effectively adds another NULLification, which is superfluous. It also adds a check for `ht`, which is also unnecessary (or not done by design decision).
Looking into the OSS-fuzz issue more closely, it looks to me that `cleanup()` has been called twice. This can only happen during fuzzing. Additionally, the cleanup_html_url() function is only called if either DEBUG_MALLOC or TESTING is set.
Anyway, this should be fixed properly to avoid cluttering fuzzing results unnecessarily.
@Shivam can you add the oss-fuzz corpus to fuzz/wget_options_fuzzer.repro/ so that "make check-valgrind" triggers the issue?A quick fix can be done cleanup_html_url(): explicitly set interesting_tags and interesting_attributes to NULL.
As Derek Martin mentions, to effectively simplify the use of hash_table_destroy() *and* to also avoid the reported failure, a better way of fixing the issue is to use a pointer to a pointer argument to allow resetting the memory referring pointer within the function itself.
Regards, Tim On 3/24/25 08:32, shivam tiwari wrote:
Dear Wget Developers, This patch fixes a Heap-use-after-free vulnerability in the hash_table_destroy function. Vulnerability Details: Project: wget Fuzzing Engine: honggfuzz Fuzz Target: wget_options_fuzzer Job Type: honggfuzz_asan_wget Platform: Linux Crash Type: Heap-use-after-free READ 8 Issue Link: https://issues.oss-fuzz.com/issues/385180607 Description of Changes: The vulnerability was caused by accessing the hash table (ht) and its cells after they had been freed. To resolve this issue, the following changes were made: Function Affected: hash_table_destroy Resolution: After freeing ht->cells and ht, they are now set to NULL to ensure they are not accessed again.By setting ht and its cells to NULL after freeing them, we prevent any further access to the freed memory, thus fixing the Heap-use-after-free vulnerability. Thanks
OpenPGP_signature.asc
Description: OpenPGP digital signature
