tests: Add fuzzing harness for BanMan #19222

practicalswift commented at 12:00 PM on June 9, 2020: contributor

Add fuzzing harness for BanMan.

See doc/fuzzing.md for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the Bitcoin Core fuzzing corpus repo.

Happy fuzzing :)

fanquake added the label Tests on Jun 9, 2020

practicalswift commented at 3:11 PM on June 9, 2020: contributor

Now fuzzing BanMan::Discourage(…) too: the commit from #19219 is now part of this PR.

DrahtBot commented at 10:47 PM on June 9, 2020: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#19415 (net: Make DNS lookup mockable, add fuzzing harness by practicalswift)
#19288 (tests: Add fuzzing harness for TorController by practicalswift)
#19259 (tests: Add fuzzing harness for LoadMempool(...) and DumpMempool(...) by practicalswift)
#19203 (net: Add regression fuzz harness for CVE-2017-18350. Add FuzzedSocket. Add thin SOCKET wrapper. by practicalswift)
#19143 (tests: Add fuzzing harnesses for CAutoFile, CBufferedFile, LoadExternalBlockFile and other FILE* consumers by practicalswift)
#19074 (tests: Use BasicTestingSetup to initialise fuzzing environment by practicalswift)

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.

DrahtBot cross-referenced this on Jun 9, 2020 from issue net: Add regression fuzz harness for CVE-2017-18350. Add FuzzedSocket. by practicalswift

DrahtBot cross-referenced this on Jun 10, 2020 from issue Replace automatic bans with discouragement filter by sipa

DrahtBot cross-referenced this on Jun 10, 2020 from issue tests: Add fuzzing harnesses for CAutoFile, CBufferedFile, LoadExternalBlockFile and other FILE* consumers by practicalswift

DrahtBot cross-referenced this on Jun 10, 2020 from issue tests: Use BasicTestingSetup to initialise fuzzing environment by practicalswift

practicalswift force-pushed on Jun 10, 2020

DrahtBot cross-referenced this on Jun 11, 2020 from issue refactor: replace CConnman pointers by references in net_processing.cpp by theStack

DrahtBot cross-referenced this on Jun 11, 2020 from issue tests: Add fuzzing harness for {Read,Write}{LE,BE}{16,32,64} (crypto/common.h) by practicalswift

DrahtBot added the label Needs rebase on Jun 11, 2020

practicalswift force-pushed on Jun 11, 2020

DrahtBot removed the label Needs rebase on Jun 11, 2020

DrahtBot cross-referenced this on Jun 13, 2020 from issue fuzz: Add fuzzing harness for LoadMempool(...) and DumpMempool(...) by practicalswift

DrahtBot cross-referenced this on Jun 16, 2020 from issue fuzz: Add fuzzing harness for TorController by practicalswift

practicalswift cross-referenced this on Jun 22, 2020 from issue Add seeds by practicalswift

DrahtBot cross-referenced this on Jun 30, 2020 from issue net: Make DNS lookup mockable, add fuzzing harness by practicalswift

DrahtBot cross-referenced this on Jul 4, 2020 from issue Cache responses to GETADDR to prevent topology leaks by naumenkogs

in src/test/fuzz/banman.cpp:38 in ad953a2ea2 outdated

  33 | +{
  34 | +    FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
  35 | +    const fs::path banlist_file = GetDataDir() / "fuzzed_banlist.dat";
  36 | +    fs::remove(banlist_file);
  37 | +    {
  38 | +        BanMan ban_man{banlist_file, nullptr, ConsumeBanTimeOffset(fuzzed_data_provider)};

Crypt-iQ commented at 8:30 AM on July 5, 2020:

@practicalswift Do you think adding random data to "fuzzed_banlist.dat", then calling BanMan ban_man{banlist_file, nullptr, ConsumeBanTimeOffset(fuzzed_data_provider)}; could be its own fuzz test? It would basically fuzz DeserializeFileDB in src/addrdb.cpp.

practicalswift commented at 9:01 PM on July 9, 2020:

That is a good idea that can be tackled in a follow-up PR, but instead of writing files to disk I think we should use a mocked/fuzzed filesystem interface like the one that will be introduced as part of #19143. Please consider reviewing that PR :)

DrahtBot added the label Needs rebase on Jul 7, 2020

tests: Add ConsumeSubNet(...). Move and increase coverage in ConsumeNetAddr(...). deba199f1c

tests: Add fuzzing harness for BanMan 97846d7f5b

practicalswift force-pushed on Jul 8, 2020

DrahtBot removed the label Needs rebase on Jul 8, 2020

practicalswift commented at 9:03 PM on July 9, 2020: contributor

Rebased on master now that @sipa's discouragement filter change in #19219 has been merged :)

Crypt-iQ commented at 5:44 AM on July 11, 2020: contributor

Tested ACK :)

Ran against ad953a2 for about 18 hours on a Ubuntu VM with 2 core and 2g RAM. It was pretty slow even for a weak VM, probably because of file io when dumping the banlist.

start_time        : 1594037321
last_update       : 1594098196
fuzzer_pid        : 16179
cycles_done       : 0
execs_done        : 379262
execs_per_sec     : 6.20
paths_total       : 341
paths_favored     : 108
paths_found       : 340
paths_imported    : 0
max_depth         : 4
cur_path          : 134
pending_favs      : 81
pending_total     : 301
variable_paths    : 329
stability         : 98.81%
bitmap_cvg        : 13.03%
unique_crashes    : 0
unique_hangs      : 0
last_path         : 1594098172
last_crash        : 0
last_hang         : 0
execs_since_crash : 379262
exec_timeout      : 320
afl_banner        : banman
afl_version       : 2.57b
target_mode       : no_forksrv 
command_line      : afl/afl-fuzz -i inputs/ -o outputs/ -m500 -- src/test/fuzz/banman
slowest_exec_ms   : 320
peak_rss_mb       : 53

Coverage for banman.cpp here: https://crypt-iq.github.io/btc/fuzz-cov/src/banman.cpp.gcov.html

in src/test/fuzz/util.h:26 in 97846d7f5b

  21 |  #include <test/fuzz/FuzzedDataProvider.h>
  22 |  #include <test/fuzz/fuzz.h>
  23 | +#include <test/util/setup_common.h>
  24 |  #include <txmempool.h>
  25 |  #include <uint256.h>
  26 |  #include <version.h>

MarcoFalke commented at 9:39 AM on July 11, 2020:

instead of having a fuzz header which is included in all fuzz test, and itself includes all of bitcoin core, what about moving the implementations to a cpp file or maybe even a fuzz/util/net etc

practicalswift commented at 5:23 PM on July 11, 2020:

Sounds like a plan! :)

MarcoFalke merged this on Jul 11, 2020

MarcoFalke closed this on Jul 11, 2020

practicalswift commented at 5:38 PM on July 11, 2020: contributor

@Crypt-iQ Thanks a lot for testing and reviewing. Great to have you on board as one of the actively reviewing fuzzing enthusiasts of the project! :)

sidhujag referenced this in commit f9e54506a0 on Jul 11, 2020

deadalnix referenced this in commit b50cc801cf on Feb 3, 2021

practicalswift deleted the branch on Apr 10, 2021

kwvg referenced this in commit 95e93c5b9d on May 7, 2022

kwvg referenced this in commit 8e66ef0a65 on May 7, 2022

kwvg referenced this in commit b36b3743d1 on Jun 14, 2022

kwvg referenced this in commit d2cd32b51e on Jun 14, 2022

kwvg referenced this in commit b630c906f0 on Jun 14, 2022

kwvg referenced this in commit 38305d5d3d on Jun 18, 2022

kwvg referenced this in commit ca2ddd9477 on Jun 18, 2022

kwvg referenced this in commit cd470e9fba on Jul 4, 2022

kwvg referenced this in commit 8714204272 on Jul 4, 2022

kwvg referenced this in commit 179f6f5ef2 on Jul 6, 2022

kwvg referenced this in commit 0942d89c81 on Jul 6, 2022

kwvg referenced this in commit 509d19456f on Jul 6, 2022

kwvg referenced this in commit 81af5029e2 on Jul 13, 2022

kwvg referenced this in commit a32ff13f4b on Jul 13, 2022

kwvg referenced this in commit da690542f4 on Jul 15, 2022

PastaPastaPasta referenced this in commit 30d6584cb6 on Jul 17, 2022

bitcoin locked this on Aug 18, 2022