Skip to content

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

@rustyrussell rustyrussell merged commit 0651b37 into ElementsProject:master Jul 8, 2025
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.

3 participants