{{ message }}
Fix Several Bugs in the fuzz_submodule Causing a lot of False Alarms in the OSS-Fuzz Bug Tracker#1950
Merged
Byron merged 6 commits intoAug 9, 2024
Conversation
Fixes a bug in the `fuzz_submodule` harness where the fuzzed data can produce file names that exceed the maximum size allowed byt the OS. This issue came up previously and was fixed in gitpython-developers#1922, but the submodule file name fixed here was missed in that PR. Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=69456
This reduces false positive test failures by identifying and gracefully handling exceptions that are explicitly raised by GitPython, thus reducing the false-positive fuzzing test failure rate.
Changes:
- `match_exception_with_traceback` uses regular expressions for more
flexible matching of file paths and line numbers. This allows for
partial matches and more complex patterns.
- Improve `check_exception_against_list` by delegating to
`match_exception_with_traceback` for checking tracebacks against
exception list entries.
- `load_exception_list`: Remains largely unchanged, as it correctly
parses the file and line number from each exception entry. However,
we ensure the set consists of regex patterns to match against
tracebacks.
Changes:
- Simplify exception handling in test harnesses via `handle_exception(e)`
in the `except Exception as e:` block.
- `setup_git_environment` is a step towards centralizing environment
variable and logging configuration set up consistently across
different fuzzing scripts. **Only applying it to a single test for
now is an intentional choice in case it fails to work in the
ClusterFuzz environment!** If it proves successful, a follow-up
change set will be welcome.
To ensure that all necessary files are included in the explicit-exceptions-list.txt file and unwanted files and directories are not.
The environment setup must happen before the `git` module is imported, otherwise GitPython won't be able to find the Git executable and raise an exception that causes the ClusterFuzz fuzzer runs to fail.
bf2a112 to
2ed3334
Compare
August 9, 2024 05:00
Byron
approved these changes
Aug 9, 2024
Byron
left a comment
Member
There was a problem hiding this comment.
Great, thanks so much!
Sorry, something went wrong.
DaveLak
commented
Aug 9, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.
Fixes the buggy
fuzz_submoduleharness which is the root cause of all recent OSS-Fuzz/Monorail issues opened.There are several distinct changes introduced here, but they are all addressing the same related exception handling weaknesses in the fuzz harness code so I think they make sense in a single PR.
Commit messages should provide relevant context, however I want to explicitly mention one change that is particularly noteworthy: the introduction of a mechanism to filter shallow errors using an explicit exceptions list.
This new pattern involves generating an 'explicit-exceptions-list.txt' by scanning for 'raise' and 'assert' statements via
git grepduring the container build step. The list helps the fuzz harness to distinguish between expected and unexpected exceptions, significantly reducing false positives.The changes I propose here are intentionally limited in scope for now to get feedback/test in prod (lol) before adopting this pattern wholesale. If successful, which I believe it will be, it should make more developing more interesting tests faster to do.
P.S. sorry for the delay on this!!!