Page MenuHomePhabricator

[backport#17357 2/2] tests: Add fuzzing harness for Bech32 encoding/decoding
ClosedPublic

Authored by majcosta on Jul 10 2020, 21:23.

Details

Summary

This ended up a bit different from Core's for obvious reasons

https://github.com/bitcoin/bitcoin/pull/17357/commits/b7541705d0abfbddf682a0134f3fa8a8e1d06cdf


Depends on D6904

Concludes backport of Core PR17357

Test Plan
cmake -GNinja .. -DENABLE_SANITIZERS="address;fuzzer" -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++
ninja bitcoin-fuzzers
./src/test/fuzz/cashaddr

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

[Bot Message]
One or more PR numbers were detected in the summary.
Links to those PRs have been inserted into the summary for reference.

majcosta retitled this revision from [backport#17357] tests: Add fuzzing harness for Bech32 encoding/decoding to [backport#17357 2/2] tests: Add fuzzing harness for Bech32 encoding/decoding.Jul 10 2020, 21:54
deadalnix requested changes to this revision.Jul 10 2020, 22:40
deadalnix added a subscriber: deadalnix.
deadalnix added inline comments.
src/test/fuzz/cashaddr.cpp
36 ↗(On Diff #22199)

Unless i don't understand something, I don't think bc is correct here.

This revision now requires changes to proceed.Jul 10 2020, 22:40

good catch, that was wrong

deadalnix requested changes to this revision.Jul 11 2020, 13:11
deadalnix added inline comments.
src/test/fuzz/cashaddr.cpp
19 ↗(On Diff #22212)

I think the : is not expected in the prefix. That should probably fail, because the prefix is expected to be letters only IIRC.

Testing for empty prefix would also be beneficial, IMO.

This revision now requires changes to proceed.Jul 11 2020, 13:11

used fuzzed input for the prefix as well, that increased coverage a bit

deadalnix requested changes to this revision.Jul 17 2020, 13:23
deadalnix added inline comments.
src/test/fuzz/cashaddr.cpp
22 ↗(On Diff #22274)

This will always be length 64, no ? It should probably be random length, or there is a whole class of errors that will not show up.

This revision now requires changes to proceed.Jul 17 2020, 13:23
majcosta marked an inline comment as done.
majcosta added inline comments.
src/test/fuzz/cashaddr.cpp
22 ↗(On Diff #22274)

Nono, that returns a string from 0 to 64, randomly

This revision is now accepted and ready to land.Jul 17 2020, 17:47
deadalnix requested changes to this revision.Jul 17 2020, 17:48
deadalnix added inline comments.
src/test/fuzz/cashaddr.cpp
22 ↗(On Diff #22274)

ok. 64 is only 320 bits, no? You might want to bump this to 112 or so so that you can get past the max length.

This revision now requires changes to proceed.Jul 17 2020, 17:48
majcosta marked an inline comment as done.

fed a longer string to the fuzzed function to increase potential coverage

This revision is now accepted and ready to land.Jul 17 2020, 22:10