Add fuzzing harness for Bech32 encoding/decoding.
Testing this PR
Run:
$ make distclean
$ ./autogen.sh
$ CC=clang CXX=clang++ ./configure --enable-fuzz \
--with-sanitizers=address,fuzzer,undefined
$ make
$ src/test/fuzz/bech32 -max_total_time=60
…
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--174a7506f384e20aa4161008e828411d-->
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
When I try to test this, make fails with:
CXXLD test/fuzz/address_deserialize
/usr/bin/ld: libbitcoin_util.a(libbitcoin_util_a-threadinterrupt.o): in function `UniqueLock<AnnotatedMixin<std::mutex>, std::unique_lock<std::mutex> >::~UniqueLock()':
/home/jon/projects/bitcoin/bitcoin/src/./sync.h:168: undefined reference to `LeaveCritical()'
clang: error: linker command failed with exit code 1 (use -v to see invocation)
Full configure/make gist here.
I have never been able to get the fuzzing to run when following the info in doc/fuzzing.md, so this may be unrelated to the PR. Any tips would be welcome.
Have you tried a make distclean before ./configure?
@jonatack What @MarcoFalke said :)
I've now updated OP with make distclean; ./autogen.sh. Will use that in the "how to test this PR template" in future OP:s for fuzzing PR:s.
$ make distclean
$ ./autogen.sh
$ CC=clang CXX=clang++ ./configure --enable-fuzz \
--with-sanitizers=address,fuzzer,undefined
$ make
$ src/test/fuzz/bech32
…
It looks like that solved it. Thanks!
How long does the test run -- until it's interrupted?
$ src/test/fuzz/bech32
INFO: Seed: 2164764235
INFO: Loaded 1 modules (506453 inline 8-bit counters): 506453 [0x559733d74988, 0x559733df03dd),
INFO: Loaded 1 PC tables (506453 PCs): 506453 [0x559733df03e0,0x5597345aa930),
INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 4096 bytes
INFO: A corpus is not provided, starting from an empty corpus
[#2](/bitcoin-bitcoin/2/) INITED cov: 740 ft: 741 corp: 1/1b lim: 4 exec/s: 0 rss: 107Mb
[#4](/bitcoin-bitcoin/4/) NEW cov: 743 ft: 853 corp: 2/3b lim: 4 exec/s: 0 rss: 107Mb L: 2/2 MS: 2 ShuffleBytes-CopyPart-
...
[#4005097](/bitcoin-bitcoin/4005097/) REDUCE cov: 829 ft: 2420 corp: 213/17Kb lim: 3381 exec/s: 2673 rss: 734Mb L: 44/641 MS: 3 CopyPart-EraseBytes-CopyPart-
[#4132416](/bitcoin-bitcoin/4132416/) REDUCE cov: 829 ft: 2420 corp: 213/17Kb lim: 3502 exec/s: 2640 rss: 734Mb L: 7/641 MS: 4 ChangeBit-ChangeBit-ShuffleBytes-EraseBytes-
Concept ACK, initial code review looks good.
@jonasnick Yes, libFuzzer runs until interrupted :)
Pass -max_total_time=60 to run the fuzzer during 60 seconds, and -help=1 to list all other libFuzzer options.
Recommended setup:
$ mkdir -p generated-test-corpus/
$ src/test/fuzz/bech32 -max_total_time=60 generated-test-corpus/
$ # now go into generated-test-corpus/ and investigate the generated test corpus :)
You might want to look at the libFuzzer documentation too if you haven't done that already :)
Thank you @practicalswift, I almost lost my mind trying to figure out libFuzzer today.
Sorry - wrong Jonas/Jon. Was meaning to mention @jonatack :)
257 | @@ -258,4 +258,6 @@ std::string ToUpper(const std::string& str); 258 | */ 259 | std::string Capitalize(std::string str); 260 | 261 | +bool CaseInsensitiveEqual(const std::string& s1, const std::string& s2);
This is only used in tests? Could move to src/test/util/str?
11 | @@ -12,6 +12,10 @@ 12 | import subprocess 13 | import logging 14 | 15 | +# Fuzzers known to lack a seed corpus in https://github.com/bitcoin-core/qa-assets/tree/master/fuzz_seed_corpus 16 | +FUZZERS_MISSING_CORPORA = [ 17 | + "bech32", 18 | +]
could remove?
@MarcoFalke Feedback addressed! Please re-review :)
9 | + 10 | +#include <cassert> 11 | +#include <cstdint> 12 | +#include <string> 13 | +#include <utility> 14 | +#include <vector>
Can remove?
+#include <cstdint>
+#include <string>
+#include <utility>
+#include <vector>
We are supposed to include what we use according to the developer notes :)
Every .cpp and .h file should #include every header file it directly uses classes, functions or other definitions from, even if those headers are already included indirectly through other headers.
Rationale: Excluding headers because they are already indirectly included results in compilation failures when those indirect dependencies change. Furthermore, it obscures what the real code dependencies are.
Oh right. Thanks for the reminder :)
ACK b7541705d0abfbddf682a0134f3fa8a8e1d06cdf
This fuzz test afaict verifies round-trip through bech32 Decode + Encode, comparing the initial random_string with the reencoded value, then Encode + Decode and asserts the correct values for the hrp (human-readable part as a string) and data.
Tested as per PR description, read code, ran tests.
((HEAD detached at origin/pr/17357))$ src/test/fuzz/bech32 -max_total_time=60
INFO: Seed: 3403448425
INFO: Loaded 1 modules (506466 inline 8-bit counters): 506466 [0x5606fb3bc9e8, 0x5606fb43844a),
INFO: Loaded 1 PC tables (506466 PCs): 506466 [0x5606fb438450,0x5606fbbf2a70),
INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 4096 bytes
INFO: A corpus is not provided, starting from an empty corpus
[#2](/bitcoin-bitcoin/2/) INITED cov: 746 ft: 747 corp: 1/1b lim: 4 exec/s: 0 rss: 106Mb
.../...
[#316157](/bitcoin-bitcoin/316157/) DONE cov: 835 ft: 2274 corp: 152/4114b lim: 122 exec/s: 5182 rss: 587Mb
###### Recommended dictionary. ######
"\xfd\xff\xff\xff" # Uses: 22198
"\x00\x00\x00\x00\x00\x00\x00\x00" # Uses: 3159
###### End of recommended dictionary. ######
Done 316157 runs in 61 second(s)
I'm seeing appveyor failures:
bech32_tests.obj : error LNK2001: unresolved external symbol "bool __cdecl CaseInsensitiveEqual
I'm seeing appveyor failures:
Appveyor error is presumably caused by this PR and fixed by #17384 (comment)