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

Fix corpus call method resolution bug, improve startup logging #308

Merged
merged 3 commits into from
Feb 28, 2024

Conversation

Xenomega
Copy link
Member

@Xenomega Xenomega commented Feb 24, 2024

This PR should close #287 . The behavior noted in that issue should not have passed through corpus call sequence replay + validation. However, there were two methods for resolving/fetching the *abi.Method associated to a CallSequenceElement/CallMessageDataAbiValues, and the logic deviated between corpus replay and fuzzer workers.

This resolved it by ensuring the CallSequenceElement.Method() function returns the CallMessageDataAbiValues.Method if it exists.

It also makes the following changes:

  • Deprecated CallMessageDataAbiValues.methodName in corpus.
    • When replaying a call sequence in the corpus, a contract would be resolved, and it's ABI would be searched through for the method by method name.
    • However, this does not support overloaded functions (same name, different signature). This is still read in and attempted to be used to resolve methods if newer methods fail (see next bullet point).
    • It should be removed after some time passes and corpuses no longer use it. It is no longer serialized. Only deserialized.
    • Replaced with a methodSignature field.
  • Added more logging to fuzzer startup to indicate where time is being spent.
  • Added a log for basic corpus health metrics (valid/invalid/total corpus call sequences and health %).
  • Reorder logging to avoid "Creating X workers" message being output after the "fuzz: elapsed[...]" message.

Note: In the future, the way I calculate active/total sequences in corpus should probably be done another way (not returned by the corpus initialize function), we can flag the corpusFile objects with an invalid (bool) field, and then add a method to loop through and "clean" them. Maybe add a --clean-corpus flag or something to remove invalid corpus items from disk 🤷

* Deprecated "methodName" in corpus (lack of support for function overloading). To be removed later, in favor of new "methodSignature" key.
* Improved fuzzer initialization logging
* Print basic metrics for corpus health on startup
* Reorder printing to avoid "Creating X workers" message after "fuzz: elapsed[...]" message.
@Xenomega Xenomega requested a review from anishnaik as a code owner February 24, 2024 05:46
@anishnaik anishnaik merged commit c0c3718 into master Feb 28, 2024
9 checks passed
@anishnaik anishnaik deleted the dev/fix-call-seq-resolution branch February 28, 2024 21:58
Leeyah-123 pushed a commit to Leeyah-123/medusa that referenced this pull request Jun 4, 2024
…c#308)

* * Fixed a corpus call method resolution bug.
* Deprecated "methodName" in corpus (lack of support for function overloading). To be removed later, in favor of new "methodSignature" key.
* Improved fuzzer initialization logging
* Print basic metrics for corpus health on startup
* Reorder printing to avoid "Creating X workers" message after "fuzz: elapsed[...]" message.

* Update corpus health log for readability

---------

Co-authored-by: anishnaik <[email protected]>
s4nsec pushed a commit that referenced this pull request Jul 9, 2024
* * Fixed a corpus call method resolution bug.
* Deprecated "methodName" in corpus (lack of support for function overloading). To be removed later, in favor of new "methodSignature" key.
* Improved fuzzer initialization logging
* Print basic metrics for corpus health on startup
* Reorder printing to avoid "Creating X workers" message after "fuzz: elapsed[...]" message.

* Update corpus health log for readability

---------

Co-authored-by: anishnaik <[email protected]>
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.

Crash after removing a function that is still in the corpus
2 participants