tests: Add fuzzing harnesses for CAutoFile, CBufferedFile, LoadExternalBlockFile and other FILE* consumers #19143

practicalswift · 2020-06-02T19:14:47Z

Add fuzzing harnesses for CAutoFile, CBufferedFile, LoadExternalBlockFile and other FILE* consumers:

Add FuzzedFileProvider which provides a FILE* interface to FuzzedDataProvider using fopencookie
Add FuzzedAutoFileProvider which provides a CAutoFile interface to FuzzedDataProvider
Add serialization/deserialization fuzzing helpers WriteToStream(…)/ReadFromStream(…)
Add fuzzing harness for CAutoFile (streams.h)
Add fuzzing harness for CBufferedFile (streams.h)
Add fuzzing harness for LoadExternalBlockFile(...) (validation.h)
Add fuzzing harness for CBlockPolicyEstimator::Read and CBlockPolicyEstimator::Write (policy/fees.h)

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 :)

DrahtBot · 2020-06-08T00:08:21Z

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

fuzz: Add fuzzing harness for LoadMempool(...) and DumpMempool(...) #19259 (tests: Add fuzzing harness for LoadMempool(...) and DumpMempool(...) by practicalswift)
net: Add regression fuzz harness for CVE-2017-18350. Add FuzzedSocket. #19203 (net: Add regression fuzz harness for CVE-2017-18350. Add FuzzedSocket. Add thin SOCKET wrapper. 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.

Crypt-iQ

utACK 27bef8c

src/test/fuzz/util.h

Crypt-iQ

Combined fuzzing coverage for only the fuzzers in this branch: https://crypt-iq.github.io/pr19143_cov/src/

Each fuzzer was run for about 24 hours. I used a variation of libFuzzer and AFL.
There seems to be some missing coverage in CBlockPolicyEstimator::Write & CBlockPolicyEstimator::Read. That could probably be fixed by better seeds. There's also missing coverage in LoadExternalBlockFile, and this can also be fixed by better seeds. I don't know the format of the external block files (I copy pasted the magic bytes from MessageStart() as seeds but no luck). This was the only slow fuzzing harness with ~5 execs/s.

src/test/fuzz/buffered_file.cpp

practicalswift · 2020-07-15T00:58:53Z

Combined fuzzing coverage for only the fuzzers in this branch: https://crypt-iq.github.io/pr19143_cov/src/

Each fuzzer was run for about 24 hours. I used a variation of libFuzzer and AFL.

Wow, that's some thorough testing! Thanks a lot!

There seems to be some missing coverage in CBlockPolicyEstimator::Write & CBlockPolicyEstimator::Read. That could probably be fixed by better seeds.

I think the perceived lack of coverage is simply due to src/test/fuzz/policy_estimator being very slow (CBlockPolicyEstimator creation is costly): I believe that given enough time the fuzzer will span the full CFG :)

To speed things up I've now added also a dedicated fuzzer for CBlockPolicyEstimator::{Write,Read} which re-uses the CBlockPolicyEstimator object across runs (without sacrificing substantial coverage stability AFAICT): see src/test/fuzz/policy_estimator_io.cpp.

This dedicated fuzzer is running at >1000 exec/s compared to <50 exec/s for the non-dedicated one.

There's also missing coverage in LoadExternalBlockFile, and this can also be fixed by better seeds. I don't know the format of the external block files (I copy pasted the magic bytes from MessageStart() as seeds but no luck). This was the only slow fuzzing harness with ~5 execs/s.

libFuzzer from LLVM 10 is able to reach past the magic bytes check within a few seconds. Perhaps you were using an old version of libFuzzer when running the load_external_block_file fuzzer?

Crypt-iQ

With clang-10 I can pass the magic bytes check (I guess it uses coverage in memcmp to solve for it?). Currently running the new policy_estimator_io fuzzer as well.

src/test/fuzz/util.h

src/test/fuzz/autofile.cpp

src/test/fuzz/buffered_file.cpp

src/test/fuzz/policy_estimator_io.cpp

…zedDataProvider using fopencookie

…e to FuzzedDataProvider

…m(…)/ReadFromStream(…)

…GetVersion} (stream.h)

…olicy/fees.h)

Crypt-iQ · 2020-07-16T14:45:28Z

Can confirm that the new policy_estimator_io fuzzer is significantly faster with libfuzzer (600-700 exec/s on one core) and hits checks in Read & Write that it otherwise doesn't hit in the policy_estimator fuzzer due to being slower. Also tried with AFL but was very slow since I forgot to compile with persistent mode :/. Will review the latest set of changes now. Coverage for just the policy_estimator_io fuzzer is here https://crypt-iq.github.io/policy_cov_llvm_outs/src/policy/fees.cpp.gcov.html

Crypt-iQ · 2020-07-18T02:33:57Z

Tested ACK ad6c348

…feredFile, LoadExternalBlockFile and other FILE* consumers ad6c348 tests: Add fuzzing harness for CBlockPolicyEstimator::{Read,Write} (policy/fees.h) (practicalswift) 614e080 tests: Add fuzzing harness for CBufferedFile::{SetPos,GetPos,GetType,GetVersion} (stream.h) (practicalswift) 7bcc71e tests: Add fuzzing harness for LoadExternalBlockFile(...) (validation.h) (practicalswift) 9823376 tests: Add fuzzing harness for CBufferedFile (streams.h) (practicalswift) f3aa659 tests: Add fuzzing harness for CAutoFile (streams.h) (practicalswift) e507c07 tests: Add serialization/deserialization fuzzing helpers WriteToStream(…)/ReadFromStream(…) (practicalswift) e48094a tests: Add FuzzedAutoFileProvider which provides a CAutoFile interface to FuzzedDataProvider (practicalswift) 9dbcd68 tests: Add FuzzedFileProvider which provides a FILE* interface to FuzzedDataProvider using fopencookie (practicalswift) Pull request description: Add fuzzing harnesses for `CAutoFile`, `CBufferedFile`, `LoadExternalBlockFile` and other `FILE*` consumers: * Add `FuzzedFileProvider` which provides a `FILE*` interface to `FuzzedDataProvider` using `fopencookie` * Add `FuzzedAutoFileProvider` which provides a `CAutoFile` interface to `FuzzedDataProvider` * Add serialization/deserialization fuzzing helpers `WriteToStream(…)`/`ReadFromStream(…)` * Add fuzzing harness for `CAutoFile` (`streams.h`) * Add fuzzing harness for `CBufferedFile` (`streams.h`) * Add fuzzing harness for `LoadExternalBlockFile(...)` (`validation.h`) * Add fuzzing harness for `CBlockPolicyEstimator::Read` and `CBlockPolicyEstimator::Write` (`policy/fees.h`) See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/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](https://github.com/bitcoin-core/qa-assets). Happy fuzzing :) ACKs for top commit: Crypt-iQ: Tested ACK ad6c348 Tree-SHA512: a38e142608218496796a527d7e59b74e30279a2815450408b7c27a76ed600cebc6b88491e831665a0639671e2d212453fcdca558500bbadbeb32b267751f8f72

…alBlockFile and other FILE* consumers Summary: Backport of core [[bitcoin/bitcoin#19143 | PR19143]]. Depends on D9131. The last commit (bitcoin/bitcoin@ad6c348) has been skipped since we don't have the policy estimator like core. Test Plan: ninja bitcoin-fuzzers ./test/fuzz/test_runner.py <path_to_corpus> Reviewers: #bitcoin_abc, majcosta Reviewed By: #bitcoin_abc, majcosta Differential Revision: https://reviews.bitcoinabc.org/D9132

…File, LoadExternalBlockFile and other FILE* consumers

backport: bitcoin#18867, bitcoin#19247, bitcoin#19222, bitcoin#18363, bitcoin#18190, bitcoin#18393, partial bitcoin#18047, bitcoin#18314, bitcoin#19143 (fuzzing harness backports: part 3)

DrahtBot added Build system Tests labels Jun 2, 2020

DrahtBot mentioned this pull request Jun 7, 2020

net: Add regression fuzz harness for CVE-2017-18350. Add FuzzedSocket. #19203

Merged

This was referenced Jun 9, 2020

tests: Use BasicTestingSetup to initialise fuzzing environment #19074

Closed

tests: Add fuzzing harness for BanMan #19222

Merged

tests: Add fuzzing harness for {Read,Write}{LE,BE}{16,32,64} (crypto/common.h) #19247

Merged

DrahtBot added the Needs rebase label Jun 11, 2020

practicalswift force-pushed the fuzzers-FILE branch from 5eda0bd to 7282135 Compare June 11, 2020 19:55

DrahtBot removed the Needs rebase label Jun 11, 2020

practicalswift force-pushed the fuzzers-FILE branch from 7282135 to 36bf1d0 Compare June 12, 2020 07:46

DrahtBot mentioned this pull request Jun 13, 2020

fuzz: Add fuzzing harness for LoadMempool(...) and DumpMempool(...) #19259

Merged

practicalswift force-pushed the fuzzers-FILE branch from 36bf1d0 to 27bef8c Compare June 15, 2020 21:30

DrahtBot mentioned this pull request Jun 16, 2020

fuzz: Add fuzzing harness for TorController #19288

Merged

practicalswift mentioned this pull request Jun 22, 2020

Add seeds bitcoin-core/qa-assets#17

Merged

DrahtBot mentioned this pull request Jun 30, 2020

net: Make DNS lookup mockable, add fuzzing harness #19415

Merged

Crypt-iQ reviewed Jul 10, 2020

View reviewed changes

src/test/fuzz/util.h Outdated Show resolved Hide resolved

DrahtBot added the Needs rebase label Jul 11, 2020

practicalswift force-pushed the fuzzers-FILE branch from 27bef8c to 36328c3 Compare July 11, 2020 18:17

DrahtBot removed the Needs rebase label Jul 11, 2020

Crypt-iQ reviewed Jul 14, 2020

View reviewed changes

src/test/fuzz/buffered_file.cpp Outdated Show resolved Hide resolved

practicalswift force-pushed the fuzzers-FILE branch 2 times, most recently from f4f0a0c to 74a31f2 Compare July 15, 2020 00:44

practicalswift force-pushed the fuzzers-FILE branch 2 times, most recently from 46ce3bf to bb87114 Compare July 15, 2020 08:43

Crypt-iQ reviewed Jul 15, 2020

View reviewed changes

practicalswift added 3 commits July 15, 2020 11:41

tests: Add FuzzedFileProvider which provides a FILE* interface to Fuz…

9dbcd68

…zedDataProvider using fopencookie

tests: Add FuzzedAutoFileProvider which provides a CAutoFile interfac…

e48094a

…e to FuzzedDataProvider

tests: Add serialization/deserialization fuzzing helpers WriteToStrea…

e507c07

…m(…)/ReadFromStream(…)

practicalswift added 4 commits July 15, 2020 11:41

tests: Add fuzzing harness for CBufferedFile (streams.h)

9823376

tests: Add fuzzing harness for LoadExternalBlockFile(...) (validation.h)

7bcc71e

tests: Add fuzzing harness for CBufferedFile::{SetPos,GetPos,GetType,…

614e080

…GetVersion} (stream.h)

tests: Add fuzzing harness for CBlockPolicyEstimator::{Read,Write} (p…

ad6c348

…olicy/fees.h)

practicalswift force-pushed the fuzzers-FILE branch from bb87114 to ad6c348 Compare July 15, 2020 11:43

maflcko merged commit 090d877 into bitcoin:master Jul 18, 2020

fanquake mentioned this pull request Jul 20, 2020

test: recently merged fuzzers don't compile on macOS #19557

Closed

LarryRuane mentioned this pull request Dec 7, 2020

util: improve FindByte() performance #19690

Merged

practicalswift deleted the fuzzers-FILE branch April 10, 2021 19:42

kwvg added a commit to kwvg/dash that referenced this pull request Jul 15, 2022

partial bitcoin#19143: Add fuzzing harnesses for CAutoFile, CBuffered…

eb2fa81

…File, LoadExternalBlockFile and other FILE* consumers

bitcoin locked as resolved and limited conversation to collaborators Aug 18, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

tests: Add fuzzing harnesses for CAutoFile, CBufferedFile, LoadExternalBlockFile and other FILE* consumers #19143

tests: Add fuzzing harnesses for CAutoFile, CBufferedFile, LoadExternalBlockFile and other FILE* consumers #19143

Uh oh!

practicalswift commented Jun 2, 2020 •

edited

Loading

Uh oh!

DrahtBot commented Jun 8, 2020 •

edited

Loading

Uh oh!

Crypt-iQ left a comment

Uh oh!

Uh oh!

Crypt-iQ left a comment

Uh oh!

Uh oh!

practicalswift commented Jul 15, 2020

Uh oh!

Crypt-iQ left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Crypt-iQ commented Jul 16, 2020 •

edited

Loading

Uh oh!

Crypt-iQ commented Jul 18, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tests: Add fuzzing harnesses for CAutoFile, CBufferedFile, LoadExternalBlockFile and other FILE* consumers #19143

tests: Add fuzzing harnesses for CAutoFile, CBufferedFile, LoadExternalBlockFile and other FILE* consumers #19143

Uh oh!

Conversation

practicalswift commented Jun 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Jun 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

Crypt-iQ left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Crypt-iQ left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

practicalswift commented Jul 15, 2020

Uh oh!

Crypt-iQ left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Crypt-iQ commented Jul 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Crypt-iQ commented Jul 18, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

practicalswift commented Jun 2, 2020 •

edited

Loading

DrahtBot commented Jun 8, 2020 •

edited

Loading

Crypt-iQ commented Jul 16, 2020 •

edited

Loading