Skip to content

Fix memory leak in fuzz_array.c caused by cupsArrayDup #8

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

AndrewJerryV
Copy link

Overview

This patch fixes a 2-byte memory leak in the existing CUPS fuzzer (fuzz_array.c). The leak occurred because cupsArrayDup() duplicates each element, but those new allocations weren’t freed before deleting the array.

Changes

  • After calling cupsArrayDup(), added a loop to iterate over every element in the duplicated array and call free() on each one.
  • Verified locally using:
    python3 infra/helper.py run_fuzzer cups fuzz_array

This change adds a loop to explicitly free the strdup'd strings inside the array returned by cupsArrayDup.

cupsArrayDup creates a shallow copy of the array and duplicates each element using strdup(), but does not associate a free callback with the new array. Without manual deallocation, these duplicated strings are leaked.

The added loop iterates over the elements in the duplicated array and frees them before deleting the array itself, ensuring proper cleanup and resolving the LeakSanitizer issue.

Tested with:
  python3 infra/helper.py run_fuzzer cups fuzz_array
@fish98 fish98 self-assigned this May 7, 2025
Copy link
Collaborator

@fish98 fish98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR! We appreciate your effort to improve the quality of our existing fuzzing harnesses. However, I am confused regarding the reason for the explicit free(elem) needed for dup_array when cupsArrayDelete() afterwards should handle element cleanup via _cupsArrayFree?

What test case triggers the fixed "2-byte leak"? Could you share the ASAN logs showing this specific leak, or directly share with the testcase that triggers the mentioned issue? Thank you

@AndrewJerryV
Copy link
Author

You're right, under normal circumstances, cupsArrayDelete() should handle element cleanup via _cupsArrayFree. However, in this case, the leak appears specifically after calling cupsArrayDup(), which uses strdup() internally to duplicate array elements. To address the issue, I explicitly freed each element in dup_array before calling cupsArrayDelete(), which resolved the leak in subsequent runs.

I've also attached the asan_log.txt for reference.

@fish98
Copy link
Collaborator

fish98 commented May 10, 2025

I've also attached the asan_log.txt for reference.

Thank you for your reply. Based on the log you provided, it appears that the alloc-dealloc-mismatch issue stems from the original allocation of the string via first_string = fuzzInput.str1;, followed by the improper free(first_string) call, while it should have been using the delete function. The root cause of the issue does not seem to be the one you mentioned. If possible, please share the specific test case that triggers the log you attached so we can investigate further.

@AndrewJerryV
Copy link
Author

Thank you for your clarification and for pointing out the root cause regarding the alloc-dealloc mismatch.
As requested, I’ve identified and isolated the test case that causes the crash. Below is the triggering input in hex format:

00000000  0a 0a 0a 2e                                       |....|
00000004

Additionally, I’ve compressed and attached the input file that caused the crash for your reference:
leak.gz

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants