fuzz: Add test/fuzz/test_runner.py and run it in travis #15295

maflcko · 2019-01-30T21:12:05Z

Can be run with ./test/fuzz/test_runner.py after building as described in doc/fuzzing.md

practicalswift · 2019-01-30T21:22:28Z

Concept ACK. Very nice!

Related: #10364 ("Feature request: Make Bitcoin libFuzzer-friendly and consider integration into the OSS-Fuzz project"). Feel free to collect the $20 000 USD bounty :-)

DrahtBot · 2019-01-30T22:30:26Z

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#15134 (tests: Add a Travis ASan/LSan/UBSan job testing in a unsigned char environment (-funsigned-char) by practicalswift)
#15063 (GUI: If BIP70 is disabled, attempt to fall back to BIP21 parsing by luke-jr)
#10443 (Add fee_est tool for debugging fee estimation code by ryanofsky)

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.

laanwj · 2019-01-31T11:57:02Z

Can you please explain what you're adding here—
Will adding fuzzing to travis make the tests non-deterministic, by randomizing it? Or does it only verify the current corpus?

maflcko · 2019-01-31T15:14:05Z

Indeed the script currently only supports running over all seeds exactly once

Sjors · 2019-02-01T17:55:57Z

Concept ACK. I can't get fuzzing to work on macOS (not that I tried hard), so having it on Travis sounds like a safe idea, if only to ensure people don't accidentally break the build.

test/fuzz/test_runner.py

doc/fuzzing.md

maflcko · 2019-02-13T22:13:08Z

Thx, done

practicalswift · 2019-02-14T08:09:15Z

utACK fa7ca8e

Let's get this merged :-)

maflcko · 2019-02-14T16:45:46Z

Would be nice if at least some person other than myself and travis could test this. Just copy-paste the commands in the readme on a linux machine and tell me your computer didn't crash or something.

jamesob · 2019-02-14T20:34:01Z

Tested changes (fa7ca8e) to doc/fuzzing.md on Linux 4.15.0-43-generic #46-Ubuntu SMP x86_64 with afl-fuzz 2.52b. Installed afl from scratch, rebuilt Bitcoin with requisite flags, and confirmed that running afl-fuzz with the transaction_deserialize works as intended.

Used new test_runner script which looks like an easy way to delegate to calling src/test/fuzz/*. Have yet to get any meaningful output back, but maybe this is intended?

$ ./test/fuzz/test_runner.py -l DEBUG ~/src/qa-assets/fuzz_seed_corpus transaction_deserialize

Fuzz targets found: ['address_deserialize', 'addrman_deserialize', 'banentry_deserialize', 'block_deserialize', 'blockheader_deserialize', 'blocklocator_deserialize', 'blockmerkleroot', 'blocktransactions_deserialize', 'blocktransactionsrequest_deserialize', 'blockundo_deserialize', 'bloomfilter_deserialize', 'coins_deserialize', 'diskblockindex_deserialize', 'inv_deserialize', 'messageheader_deserialize', 'netaddr_deserialize', 'service_deserialize', 'transaction_deserialize', 'txoutcompressor_deserialize', 'txundo_deserialize']
Fuzz targets selected: ['transaction_deserialize']

jamesob · 2019-02-14T20:52:28Z

test/fuzz/test_runner.py

+            logging.error("Must be built with libFuzzer")
+            sys.exit(1)
+    except subprocess.TimeoutExpired:
+        logging.error("subprocess timed out: Currently only libFuzzer is supported")


Tested and works when bitcoin is built with afl.

jamesob · 2019-02-14T21:08:02Z

Tested ACK fa535af. Rebuilt with libfuzzer and tested the command that previously failed with afl:

$ ./test/fuzz/test_runner.py -l DEBUG ~/src/qa-assets/fuzz_seed_corpus transaction_deserialize

Fuzz targets found: ['address_deserialize', 'addrman_deserialize', 'banentry_deserialize', 'block_deserialize', 'blockheader_deserialize', 'blocklocator_deserialize', 'blockmerkleroot', 'blocktransactions_deserialize', 'blocktransactionsrequest_deserialize', 'blockundo_deserialize', 'bloomfilter_deserialize', 'coins_deserialize', 'diskblockindex_deserialize', 'inv_deserialize', 'messageheader_deserialize', 'netaddr_deserialize', 'service_deserialize', 'transaction_deserialize', 'txoutcompressor_deserialize', 'txundo_deserialize']
Fuzz targets selected: ['transaction_deserialize']
Run transaction_deserialize with args ['/home/james/src/bitcoin/src/test/fuzz/transaction_deserialize', '-runs=1', '/home/james/src/qa-assets/fuzz_seed_corpus/transaction_deserialize']
Output: INFO: Seed: 819372946
INFO: Loaded 1 modules   (2002 inline 8-bit counters): 2002 [0x55b9cec15620, 0x55b9cec15df2),
INFO: Loaded 1 PC tables (2002 PCs): 2002 [0x55b9cec15df8,0x55b9cec1db18),
INFO:      295 files found in /home/james/src/qa-assets/fuzz_seed_corpus/transaction_deserialize
INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 52575 bytes
INFO: seed corpus: files: 295 min: 1b max: 52575b total: 491499b rss: 45Mb
#296    INITED cov: 240 ft: 938 corp: 277/475Kb exec/s: 0 rss: 106Mb
#296    DONE   cov: 240 ft: 938 corp: 277/475Kb exec/s: 0 rss: 106Mb
Done 296 runs in 0 second(s)

Worth noting that I had to bump clang from 4. to 6. in order for the -fsanitize=fuzzer flags to work.

fa535af fuzz: test_runner: Better error message when built with afl (MarcoFalke) fa7ca8e qa: Add test/fuzz/test_runner.py (MarcoFalke) Pull request description: Can be run with `./test/fuzz/test_runner.py` after building as described in `doc/fuzzing.md` Tree-SHA512: f6a3cd8165ec2de4b363be4fd0a936b4a60829cce923f93fe5d6a046b1bbd64c959cdf790440bf70c0e13b0bb1b956a746a24c6fd92bddeab15b837ed50ffad2

ryanofsky · 2019-02-22T16:25:00Z

src/Makefile.test.include

@@ -28,6 +27,8 @@ FUZZ_TARGETS = \

 if ENABLE_FUZZ
 noinst_PROGRAMS += $(FUZZ_TARGETS:=)
+else
+bin_PROGRAMS += test/test_bitcoin


In commit "qa: Add test/fuzz/test_runner.py" (fa7ca8e)

@MarcoFalke Is this no longer building test_bitcoin if ENABLE_FUZZ is true? I wouldn't expect ENABLE_FUZZ to affect this, but if this is correct it'd be helpful to have a comment here explaining what this is doing.

Yes, I think there is no reason to build test_bitcoin if you want to build the fuzz tests. Am I missing something?

Regardless of that, it would result in linker errors later in the build process, since you can't link libfuzzer to something that has a main function.

Oh ok, I don't know really anything about fuzzing, and I assumed enabling a fuzzing option would just build some new binaries, not affect existing binaries in any way. But I guess due to autotools lack of support for side-by-side build configurations, ENABLE_FUZZ affects every existing binary we build, as well as building new binaries? Probably not worth adding a comment here just for test_bitcoin in this case.

Yes, ideally it should disable all other binaries.

Currently we hack around that by specifying it manually:

bitcoin/.travis.yml

Line 146 in 169dced

BITCOIN_CONFIG="--disable-wallet --disable-bench --with-utils=no --with-daemon=no --with-libs=no --with-gui=no --enable-fuzz --with-sanitizers=fuzzer,address CC=clang CXX=clang++"

It is surprising that --enable-fuzz silently stops building the test binary, especially as config.log continues to claim that test_bitcoin is being built.

It is surprising that --enable-fuzz silently stops building the test binary

Might be fixed by #19388, which is blocked on #20560 . Though when building and linking with a fuzz engine, the test_bitcoin binary won't have any meaning. Not sure if it will even compile/link.

config.log continues to claim that test_bitcoin is being built.

I don't really understand build systems, so if this is an issue, I hope that someone will fix it

I'll open a PR to change config to output something like

configure:27478: checking whether to build test_bitcoin-qt configure:27481: result: no, because fuzzing is enabled configure:27499: checking whether to build test_bitcoin configure:27502: result: no, because fuzzing is enabled

It's not really surprising that enabling fuzzing would break all the other binaries, I was just misled by the config output.

d059544 [Build] fuzz target, change LIBBITCOIN_ZEROCOIN link order. (furszy) 2396e6b [fuzz] Add ContextualCheckTransaction call to transaction target. (furszy) f0887a0 Fuzzing documentation "PIVX-fication" (furszy) 9631f46 [doc] add sanitizers documentation in developer-notes.md (furszy) 70a0ace tests: Test serialisation as part of deserialisation fuzzing. Test round-trip equality where possible. Avoid code repetition. (practicalswift) e1b92b6 ignore new fuzz targets gitignore (furszy) d058d8c tests: Add deserialization fuzzing harnesses (furszy) e1f666c tests: Remove TRANSACTION_DESERIALIZE (replaced by transaction fuzzer) (practicalswift) b5f291c tests: Add fuzzing harness for CheckTransaction(...), IsStandardTx(...) and other CTransaction related functions (furszy) 3205871 fuzz: Remove option --export_coverage from test_runner (MarcoFalke) 52693ee fuzz: Add option to merge input dir to test runner (MarcoFalke) 2b4f8aa doc: Remove --disable-ccache from docs (MarcoFalke) b54b1d6 tests: Improve test runner output in case of target errors (practicalswift) cd6134f test: Log output even if fuzzer failed (MarcoFalke) 48cd0c8 doc: Improve fuzzing docs for macOS users (Fabian Jahr) d642b67 [Build] Do not disable wallet when fuzz is enabled. (furszy) c3447b5 Update doc and CI config (qmma) 1266d3e Disable other targets when enable-fuzz is set (qmma) f28ac9a build: Allow to configure --with-sanitizers=fuzzer (MarcoFalke) 425742c fuzz: test_runner: Better error message when built with afl (MarcoFalke) 541f442 qa: Add test/fuzz/test_runner.py (MarcoFalke) 89fe5b2 Add missing LIBBITCOIN_ZMQ to test target (furszy) 58dbe79 add fuzzing binaries to gitignore. (furszy) 393a126 fuzz: Move deserialize tests to test/fuzz/deserialize.cpp (MarcoFalke) a568df5 test: Build fuzz targets into separate executables (furszy) d5dddde [test] fuzz: make test_one_input return void (MarcoFalke) 2e4ec58 [fuzzing] initialize chain params by default. (furszy) 08d8ebe [tests] Add libFuzzer support. (practicalswift) 84f72da [test] Speed up fuzzing by ~200x when using afl-fuzz (practicalswift) faf2be6 Init ECC context for test_bitcoin_fuzzy. (Gregory Maxwell) 11150df Make fuzzer actually test CTxOutCompressor (Pieter Wuille) d6f6a85 doc: Add bare-bones documentation for fuzzing (Wladimir J. van der Laan) 5c3b550 Simple fuzzing framework (pstratem) Pull request description: As the title says, adding fuzzing framework support so we can start getting serious on this area as well. Adapted the following PRs: * bitcoin#9172. * bitcoin#9354. * bitcoin#9691. * bitcoin#10415. * bitcoin#10440. * bitcoin#15043. * bitcoin#15047. * bitcoin#15295. * bitcoin#15399 (fabcfa5 only). * bitcoin#16338. * bitcoin#17051. * bitcoin#17076. * bitcoin#17225. * bitcoin#17942. * bitcoin#16236 (only fa35c42). * bitcoin#18166 (only f2472f6). * bitcoin#18300. * And.. probably will go further and continue adapting more PRs.. ACKs for top commit: random-zebra: utACK d059544 and merging... Tree-SHA512: c0b05bca47bf99bafd8abf1453c5636fe05df75f16d0e9c750368ea2aed8142f0b28d28af1d23468b8829188412a80fd3b7bdbbda294b940d78aec80c1c7d03a

merge bitcoin#15043, bitcoin#15295, bitcoin#15399, bitcoin#17452, bitcoin#16338, bitcoin#15504, bitcoin#17076, bitcoin#17069, bitcoin#17018: fuzzing

maflcko force-pushed the Mf1901-qaFuzz branch from d68255d to 5a1806c Compare January 30, 2019 21:18

maflcko force-pushed the Mf1901-qaFuzz branch 4 times, most recently from 032ac70 to 4da6a56 Compare January 30, 2019 22:27

maflcko force-pushed the Mf1901-qaFuzz branch 2 times, most recently from 1074dc1 to 0ca2db9 Compare January 30, 2019 22:46

fanquake added the Tests label Jan 30, 2019

maflcko force-pushed the Mf1901-qaFuzz branch from 0ca2db9 to fa599f8 Compare January 30, 2019 23:14

maflcko added Needs gitian build and removed Needs gitian build labels Jan 31, 2019

DrahtBot removed the Needs gitian build label Feb 1, 2019

DrahtBot added the Needs rebase label Feb 1, 2019

bitcoin deleted a comment from DrahtBot Feb 2, 2019

maflcko force-pushed the Mf1901-qaFuzz branch from fa599f8 to faa77d7 Compare February 2, 2019 20:03

DrahtBot removed the Needs rebase label Feb 2, 2019

practicalswift reviewed Feb 4, 2019

View reviewed changes

test/fuzz/test_runner.py Outdated Show resolved Hide resolved

practicalswift reviewed Feb 4, 2019

View reviewed changes

test/fuzz/test_runner.py Outdated Show resolved Hide resolved

practicalswift reviewed Feb 4, 2019

View reviewed changes

test/fuzz/test_runner.py Show resolved Hide resolved

maflcko force-pushed the Mf1901-qaFuzz branch from faa77d7 to fa1d155 Compare February 5, 2019 21:46

practicalswift reviewed Feb 5, 2019

View reviewed changes

test/fuzz/test_runner.py Outdated Show resolved Hide resolved

maflcko force-pushed the Mf1901-qaFuzz branch 3 times, most recently from 0491dcb to 03ba30a Compare February 11, 2019 20:12

laanwj reviewed Feb 13, 2019

View reviewed changes

test/fuzz/test_runner.py Outdated Show resolved Hide resolved

laanwj reviewed Feb 13, 2019

View reviewed changes

doc/fuzzing.md Outdated Show resolved Hide resolved

qa: Add test/fuzz/test_runner.py

fa7ca8e

maflcko force-pushed the Mf1901-qaFuzz branch from fa7ce81 to fa7ca8e Compare February 13, 2019 22:12

DrahtBot added the Needs rebase label Feb 14, 2019

fuzz: test_runner: Better error message when built with afl

fa535af

jamesob reviewed Feb 14, 2019

View reviewed changes

DrahtBot removed the Needs rebase label Feb 14, 2019

maflcko merged commit fa535af into bitcoin:master Feb 14, 2019

maflcko deleted the Mf1901-qaFuzz branch February 14, 2019 21:35

ryanofsky reviewed Feb 22, 2019

View reviewed changes

ryanofsky mentioned this pull request Feb 22, 2019

Add fee_est tool for debugging fee estimation code #10443

Closed

maflcko mentioned this pull request May 30, 2019

--enable-fuzz fails in combination with --enable-wallet, --with-miniupnpc or --enable-zmq #16094

Closed

furszy mentioned this pull request Mar 22, 2021

Fuzzing framework support PIVX-Project/PIVX#2252

Merged

kwvg added a commit to kwvg/dash that referenced this pull request Aug 2, 2021

bitcoin#15295: Add test/fuzz/test_runner.py and run it in travis

4acb388

kwvg added a commit to kwvg/dash that referenced this pull request Aug 5, 2021

bitcoin#15295: Add test/fuzz/test_runner.py and run it in travis

07c505d

kwvg added a commit to kwvg/dash that referenced this pull request Aug 5, 2021

bitcoin#15295: Add test/fuzz/test_runner.py and run it in travis

122ac8a

PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Aug 6, 2021

bitcoin#15295: Add test/fuzz/test_runner.py and run it in travis

05f477e

kwvg added a commit to kwvg/dash that referenced this pull request Aug 8, 2021

bitcoin#15295: Add test/fuzz/test_runner.py and run it in travis

21293ce

kwvg added a commit to kwvg/dash that referenced this pull request Aug 11, 2021

bitcoin#15295: Add test/fuzz/test_runner.py and run it in travis

d967c39

5tefan pushed a commit to 5tefan/dash that referenced this pull request Aug 12, 2021

bitcoin#15295: Add test/fuzz/test_runner.py and run it in travis

cf41a16

bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022

fuzz: Add test/fuzz/test_runner.py and run it in travis #15295

fuzz: Add test/fuzz/test_runner.py and run it in travis #15295

Uh oh!

Conversation

maflcko commented Jan 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

practicalswift commented Jan 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Jan 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

laanwj commented Jan 31, 2019

Uh oh!

maflcko commented Jan 31, 2019

Uh oh!

Sjors commented Feb 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maflcko commented Feb 13, 2019

Uh oh!

practicalswift commented Feb 14, 2019

Uh oh!

maflcko commented Feb 14, 2019

Uh oh!

jamesob commented Feb 14, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jamesob commented Feb 14, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

maflcko commented Jan 30, 2019 •

edited

Loading

practicalswift commented Jan 30, 2019 •

edited

Loading

DrahtBot commented Jan 30, 2019 •

edited

Loading

Sjors commented Feb 1, 2019 •

edited

Loading