Skip to content
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

[gitpython] Fix Broken Build #11763

Closed
wants to merge 2 commits into from
Closed

Conversation

DaveLak
Copy link
Contributor

@DaveLak DaveLak commented Mar 31, 2024

Fixes ClusterFuzz issues 67399 and 55299

Issue 67399: gitpython: Fuzzing build failure

Since: 2024-03-11

The Problem

The pre-installed version of pip (19.2.3) was outdated and unable to parse the pyproject.toml syntax during the install step in build.sh causing the script to error out and crash.

The Solution

Upgrading pip to the latest version in the project image resolves the issue and allows the installation to complete.

Issue 55299: gitpython: Coverage build failure

Since: 2023-01-21

The Problem

(my hypothesis at least)

I believe the root of the issue here was caused by fuzzer initialization and execution taking too long for the actual run to generate a meaningful corpus. I suspect this because:

  • The build log posted in the CF issue shows the coverage build fails after trying to unzip the corpus archives and warning the zipfiles are empty.
  • I downloaded a backup of the corpus zips to confirm and they are in fact empty.
  • This project has a deep dependency graph which caused the call to
    atheris.instrament_all() to instrument 4,000+ functions before the fuzzer execution could begin which was causing a significant delay (on my local machine, at least) before actual test execution would start.

The Solution(-ish)

The commit message on 908ba9c should sum it up, but the TL;DR is I reduced the scope of instrumented functions to align closer with the APIs being fuzzed and added dictionaries and seed corpra which provided promising results locally.

fuzz_tree.py is still slow as far as average_exec_per_sec, but startup is quicker and with the seed data it gets close to its coverage depth fairly quickly as well.

  • I believe this will fix the OSS-Fuzz coverage build at least.
  • but I suspect the Introspector build will continue to timeout because I couldn't figure out how to get pycg to play nicely with the dependency graph
    • (I think there's a circular dependency it gets stuck on, but that's a problem for another day.)

DaveLak added 2 commits March 30, 2024 10:47
The pre-installed version of `pip` (19.x) was outdated and unable to
parse the `pyproject.toml` syntax during the install step in `build.sh`
causing the script to error out and crash.

Upgrading `pip` to the latest version in the project image resolves the
issue and allows the installation to complete.

[1]: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=67399
This project has a deep dependency graph which caused the call to
atheris.instrament_all() slow down the fuzzers significantly as it
instramented 4,000+ functions, many of which were not relevant to the
specific fuzz target APIs. Replacing it with a with-statement that only
instraments the target code provides significant efficency benefits in
startup time and execution speed.

Dictionaries and seed_corpus zips provide additional efficiency boosts.

This was made evident by the new exceptions raised by the fuzz input
data which are now handled accordingly.
Copy link

DaveLak is a new contributor to projects/gitpython. The PR must be approved by known contributors before it can be merged.

Copy link
Contributor

@Byron Byron left a comment

Choose a reason for hiding this comment

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

As I wasn't aware fuzzing was happening here, I am not sure if I am able to approve this PR. In any case, it's appreciated to fuzzing back to work - thanks a lot for your help.

@DaveLak
Copy link
Contributor Author

DaveLak commented Apr 2, 2024

Closing this in favor of a forthcoming PR to migrate the fuzz harnesses upstream.

@DaveLak DaveLak closed this Apr 2, 2024
DaveLak added a commit to DaveLak/GitPython that referenced this pull request Apr 12, 2024
Migrates the OSS-Fuzz tests and setup scripts from the OSS-Fuzz
repository to GitPython's repo as discussed here:
gitpython-developers#1887 (comment)

These files include the changes that were originally proposed in:
google/oss-fuzz#11763

Additional changes include:
- A first pass at documenting the contents of the fuzzing set up in a
  dedicated README.md
- Adding the dictionary files to this repo for improved visibility. Seed
  corpra zips are still located in an external repo pending further
 discussion regarding where those should live in the long term.
@DaveLak DaveLak deleted the fix-gitpython-build branch May 1, 2024 00:18
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