Skip to content

fuzz-tests: Improve the fuzz-base32-64 test #8183

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 2 commits into
base: master
Choose a base branch
from

Conversation

Chand-ra
Copy link

Currently, fuzz testing for b64_encode() in the fuzz/fuzz-base32-64 test merely
encodes input and frees the result, providing no real verification of its behavior.

Introduce a new b64_decode() function and update the fuzz test to perform a
roundtrip—encoding followed by decoding—to ensure that b64_encode()
correctly preserves the original data.

Add the newly discovered inputs that result in greater code-coverage to the
seed corpus for this test.

Copy link
Contributor

@morehouse morehouse left a comment

Choose a reason for hiding this comment

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

This PR is broken. Running on the existing corpus triggers a UBSan error:

tests/fuzz/fuzz-base32-64.c:25:2: runtime error: null pointer passed as argument 1, which is declared to never be null

Copy link
Contributor

@morehouse morehouse left a comment

Choose a reason for hiding this comment

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

Code changes LGTM.

I think the Changelog-Added in the first commit should be a Changelog-None though, since this is a test-only change.

Also, did you use the run.py script to minimize the number of new inputs added to the corpus?

Changelog-None: Currently, fuzz testing for b64_encode() merely
encodes input and frees the result, providing no real verification
of its behavior.

Introduce a new b64_decode() function (modeled after b32_decode())
and update the fuzz test to perform a roundtrip—encoding followed
by decoding—to ensure that b64_encode() correctly preserves the
original data.
@Chand-ra
Copy link
Author

I think the Changelog-Added in the first commit should be a Changelog-None though, since this is a test-only change.

I've made this change in the latest push.

Also, did you use the run.py script to minimize the number of new inputs added to the corpus?

Yes.

Copy link
Contributor

@morehouse morehouse left a comment

Choose a reason for hiding this comment

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

ACK 1942821

I'm guessing changelog entries need to be all on one line to be automatically parsed correctly, but in this case it's a Changelog-None, so it probably doesn't matter.

Change in the fuzz-testing scheme of fuzz-base32-64 led to
the discovery of test inputs that result in greater in
code-coverage. Add these inputs to the test's seed corpus.
@Chand-ra
Copy link
Author

The seed corpus additions are now rectified to be minimal.

@morehouse
Copy link
Contributor

ACK b4119e3

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