Resurrect pstratem's "Simple fuzzing framework" #9172

laanwj · 2016-11-16T10:33:45Z

Because I feel strongly that we should have it, this brings back #7940, with some minor changes to make it ready for merge:

Rebase for removal of CDataStream::GetVersion
Move to src/tests, rename to test_bitcoin_fuzzy to make it clear it is part of the tests, not a user entry point
Make sure that it doesn't get installed on make install nor shipped as part of the distribution (it is kind of useless without instrumentation). I opted to make it build by default with --enable-tests because if it is a specific option, no one will ever bother to enable it (which means it doesn't get built and runs out of date), and we don't want too many --enable-X anyway.

jonasschnelli · 2016-11-16T10:35:57Z

Concept ACK.
Agree that a simple documentation would be useful.
I guess right now you have to pipe in some data that will be used for the fuzz tests?

laanwj · 2016-11-16T10:38:16Z

I guess right now you have to pipe in some data that will be used for the fuzz tests?

Piping in data is the part that the fuzzer should do :)

laanwj · 2016-11-16T11:27:15Z

I've added some bare-bones instructions on how to use AFL with Bitcoin Core. What I don't have are example inputs, so this doesn't do anything yet at the moment.

@pstratem Would you mind sharing some of your example inputs?

maflcko · 2016-11-16T12:11:04Z

Concept ACK. Would prefer to see at least one MWE.

laanwj · 2016-11-16T12:44:25Z

What's an MWE?

maflcko · 2016-11-16T12:59:09Z

Minimal working example: Just a code bit that is of the smallest size, but still demonstrates how the framework is used.

laanwj · 2016-11-16T13:04:53Z

Minimal working example: Just a code bit that is of the smallest size, but still demonstrates how the framework is used.

Agreed. I demonstrate how to use the framework in the documentation I added, but there need to be some example inputs to put in inputs/ or it croaks. I could make a few but I'd rather wait for Patrick a bit whether he already has some as it's quite time-consuming.

pstratem · 2016-11-19T05:05:38Z

@laanwj suggestion for where examples should go?

(which directory)

laanwj · 2016-11-19T08:45:21Z

Don't know. Probably a special repo with bitcoin core "manual testing" data eventually. At this point though I'd be happy with a dropbox link as I'd just like to start fuzzing.

gmaxwell · 2016-11-19T10:48:21Z

FWIW, I have a 24 core host grinding on this in AFL for most of a day on this... I guess I'll leave it running all weekend and spin up some more cores. if someone wants the resulting vectors after I cmin them, I'd be glad to hand them out.

(I just started from some particularly dumb /dev/urandom starting points... a future improvement I would suggest (not this PR!) might be a command-line argument that makes it serialize a dummy object or few for each of the modes and write out a set of files.)

laanwj · 2016-11-21T07:54:05Z

a future improvement I would suggest (not this PR!) might be a command-line argument that makes it serialize a dummy object or few for each of the modes and write out a set of files.

Yes that would be useful. Should ideally have realistic examples that come up in bitcoind itself (just grabbed from the wire randomly and saved) along with synthetic ones.

if someone wants the resulting vectors after I cmin them, I'd be glad to hand them out.

Yes please!

fanquake · 2016-11-23T07:49:01Z

doc/fuzzing.md

+`AFLPATH` was set as above):
+```
+./configure --disable-ccache --disable-shared --enable-tests CC=${AFLPATH}/afl-gcc CXX=${AFLPATH}/afl-g++
+export AFL_HARDEN=1


For anyone wondering what AFL_HARDEN does. From the afl README.

Setting AFL_HARDEN=1 when calling 'make' will cause the CC wrapper to automatically enable code hardening options that make it easier to detect simple memory bugs. Libdislocator, a helper library included with AFL (see libdislocator/README.dislocator) can help uncover heap corruption issues, too.

It may actually be unnecessary with bitcoin core which already enables a lot of hardening options. Then again, it can't hurt either I think.

fanquake · 2016-11-24T13:09:59Z

Started testing this on OS X 10.12.
afl compiles fine, and bitcoin configure completes (using afl-clang and afl-clang++ instead of afl-gcc and afl-gcc++):

Options used to compile and link:
  with wallet   = yes
  with gui / qt = yes
    qt version  = 5
    with qr     = yes
  with zmq      = yes
  with test     = yes
  with bench    = yes
  with upnp     = yes
  debug enabled = no

  target os     = darwin
  build os      = darwin

  CC            = /Users/xxx/github/bitcoin/afl-2.35b/afl-clang
  CFLAGS        = -g -O2
  CPPFLAGS      =  -DHAVE_BUILD_INFO -D__STDC_FORMAT_MACROS -I/usr/local/opt/berkeley-db4/include -DMAC_OSX
  CXX           = /Users/xxx/github/bitcoin/afl-2.35b/afl-clang++ -std=c++11
  CXXFLAGS      = -g -O2 -Wall -Wextra -Wformat -Wformat-security -Wno-unused-parameter -Wno-self-assign -Wno-unused-local-typedef -Wno-deprecated-register
  LDFLAGS       =  -Wl,-headerpad_max_install_names -Wl,-dead_strip

test_bitcoin_fuzzy compiles with one warning

bash-3.2$ make test/test_bitcoin_fuzzy
  CXX      test/test_test_bitcoin_fuzzy-test_bitcoin_fuzzy.o
afl-cc 2.35b by <lcamtuf@google.com>
afl-as 2.35b by <lcamtuf@google.com>
[+] Instrumented 2777 locations (64-bit, hardened mode, ratio 100%).
  CXX      lib/libunivalue_la-univalue.lo
... SNIP ...
afl-cc 2.35b by <lcamtuf@google.com>
afl-as 2.35b by <lcamtuf@google.com>
[!] WARNING: No instrumentation targets found.
  CXX      libbitcoin_util_a-random.o
... SNIP ...
afl-cc 2.35b by <lcamtuf@google.com>
afl-as 2.35b by <lcamtuf@google.com>
[+] Instrumented 48 locations (64-bit, hardened mode, ratio 100%).
  AR       crypto/libbitcoin_crypto.a
  CC       src/libsecp256k1_la-secp256k1.lo
afl-cc 2.35b by <lcamtuf@google.com>
afl-as 2.35b by <lcamtuf@google.com>
[+] Instrumented 903 locations (64-bit, hardened mode, ratio 100%).
  CCLD     libsecp256k1.la
  CXXLD    test/test_bitcoin_fuzzy

However, running gave:

bash-3.2$ $AFLPATH/afl-fuzz -i ${AFLIN} -o ${AFLOUT} -- test/test_bitcoin_fuzzy
afl-fuzz 2.35b by <lcamtuf@google.com>
[+] You have 8 CPU cores and 3 runnable tasks (utilization: 38%).
[+] Try parallel jobs - see /usr/local/share/doc/afl/parallel_fuzzing.txt.
[*] Setting up output directories...
[+] Output directory exists but deemed OK to reuse.
[*] Deleting old session data...
[+] Output dir cleanup successful.
[*] Scanning '/Users/xxx/github/bitcoin/src/inputs'...
[+] No auto-generated dictionary tokens to reuse.
[*] Creating hard links for all input files...
[*] Validating target binary...
[*] Attempting dry run with 'id:000000,orig:auto_000000'...
[*] Spinning up the fork server...

[-] Hmm, looks like the target binary terminated before we could complete a
    handshake with the injected code. There are two probable explanations:

    - The current memory limit (50.0 MB) is too restrictive, causing an OOM
      fault in the dynamic linker. This can be fixed with the -m option. A
      simple way to confirm the diagnosis may be:

[-] PROGRAM ABORT : Fork server handshake failed
         Location : init_forkserver(), afl-fuzz.c:2247

Same result even after upping the memory limit to 4GB (-m4096) which should be more than enough.

Setting AFL_NO_FORKSRV=1 seems to have made it run. From the readme:

Setting AFL_NO_FORKSRV disables the forkserver optimization, reverting to
    fork + execve() call for every tested input. This is useful mostly when
    working with unruly libraries that create threads or do other crazy
    things when initializing (before the instrumentation has a chance to run).

So for now it's running:

Also testing on Ubuntu 16.04 running in VirtualBox.

Options used to compile and link:
  with wallet   = yes
  with gui / qt = yes
    qt version  = 5
    with qr     = auto
  with zmq      = yes
  with test     = yes
  with bench    = yes
  with upnp     = yes
  debug enabled = no

  target os     = linux
  build os      = 

  CC            = /home/ubuntu/bitcoin/afl-2.35b/afl-gcc
  CFLAGS        = -g -O2
  CPPFLAGS      =  -DHAVE_BUILD_INFO -D__STDC_FORMAT_MACROS
  CXX           = /home/ubuntu/bitcoin/afl-2.35b/afl-g++ -std=c++11
  CXXFLAGS      = -g -O2 -Wall -Wextra -Wformat -Wformat-security -Wno-unused-parameter
  LDFLAGS       =

fanquake · 2016-11-24T13:37:49Z

Given that one was working alright, I've moved to running five in parallel. Will leave them running for a while. You can use the afl-whatsup tool to watch the progress, sample output below:

$AFLPATH/afl-whatsup sync_dir
status check tool for afl-fuzz by <lcamtuf@google.com>

Individual fuzzers
==================

>>> fuzzer01 (1 days, 0 hrs) <<<

  cycle 1, lifetime speed 159 execs/sec, path 744/1201 (61%)
  pending 52/1043, coverage 6.22%, no crashes yet

>>> fuzzer02 (1 days, 0 hrs) <<<

  cycle 80, lifetime speed 179 execs/sec, path 346/1199 (28%)
  pending 0/0, coverage 6.22%, no crashes yet

>>> fuzzer03 (1 days, 0 hrs) <<<

  cycle 80, lifetime speed 181 execs/sec, path 504/1190 (42%)
  pending 0/0, coverage 6.22%, no crashes yet

>>> fuzzer04 (1 days, 0 hrs) <<<

  cycle 83, lifetime speed 181 execs/sec, path 1180/1190 (99%)
  pending 0/0, coverage 6.22%, no crashes yet

>>> fuzzer05 (0 days, 1 hrs) <<<

  cycle 3, lifetime speed 146 execs/sec, path 621/1206 (51%)
  pending 0/687, coverage 6.22%, no crashes yet

Summary stats
=============

       Fuzzers alive : 5
      Total run time : 4 days, 4 hours
         Total execs : 63 million
    Cumulative speed : 846 execs/sec
       Pending paths : 52 faves, 1730 total
  Pending per fuzzer : 10 faves, 346 total (on average)
       Crashes found : 0 locally unique

maflcko · 2016-11-25T12:37:57Z

doc/fuzzing.md

+examples that it found.
+
+```
+mkdir inputs


µnit:

mkdir test/afl-inputs AFLIN=$PWD/test/afl-inputs ...

nit: we could also add .gitignore entries for test/inputs, test/outputs and test/sync_dir

Well, this is just an example. There's no strong reason to make these directories inside the bitcoin repository.

maflcko · 2016-11-25T12:38:27Z

Concept ACK 18b4c6c

maflcko · 2016-11-25T14:01:26Z

doc/fuzzing.md

+
+To start the actual fuzzing use:
+```
+$AFLPATH/afl-fuzz -i ${AFLIN} -o ${AFLOUT} -- test/test_bitcoin_fuzzy


I think the default memory needs to be increased to prevent OOM?

Which OS did you see out of memory errors on?
What did you need to increase it to to make it run?

I used the inputs by @laanwj (bitcoin_fuzzy_in.tar.xz) and the default of -m50 caused a failure in the dry run.

Setting -m52 seems to be enough in this case. (OS: fedora 24)

laanwj · 2016-11-25T14:14:05Z

I didn't have to change anything memory related on ubuntu at least

fanquake · 2016-11-25T14:15:46Z

Some results. The "Master" instance is yet to complete a cycle (~62%), and the exec speed seems to have dropped off in the last few hours. Current stats:

The other instances all have similar output, and have run through ~80cycles:

theuni · 2016-11-25T20:39:23Z

tested ACK. Works well on OSX with AFL_NO_FORKSRV set (probably worth documenting that it's necessary there).

Also, feel free to take this as a simplification: theuni@88b8f92

laanwj · 2016-11-26T12:32:55Z

@cfields thanks - though I'm not sure moving addrman to common is the right thing to do. It is decidedly a server thing and not used by any of the other (nontest) executables. And later on we may want to fuzz more things in _server.

pstratem · 2016-11-27T05:03:13Z

@fanquake if you completely remove the memory limit there are a few tests which can use nearly unbounded amounts of memory

I suggest running with:

export AFL_SKIP_CRASHES=1;
export AFL_PARAMETERS="-t 1000+ -m 1024";

afl-fuzz $AFL_PARAMETERS -i fuzzing/input -o fuzzing/output ./bitcoin/src/test/test_bitcoin_fuzzy

fanquake · 2016-11-27T14:26:19Z

@pstratem Thanks for the suggestions. I've restarted some of the fuzzers with the new vars.

fanquake · 2016-11-29T12:49:41Z

Wrapping up my testing of this. Results from individual fuzzers are below, and the sync_dir (outputs) as well as the inputs used are available for download.
inputs.zip
sync_dir.zip
Fuzzer 01 (deterministic tests):

Fuzzer 02:

Fuzzer 03:

Fuzzer 04:

Fuzzer 05 (restarted to up allocated memory and pass -t 1000):

Total Output:

$AFLPATH/afl-whatsup sync_dir
status check tool for afl-fuzz by <lcamtuf@google.com>

Individual fuzzers
==================

>>> fuzzer01 (4 days, 23 hrs) <<<

  cycle 2, lifetime speed 114 execs/sec, path 297/1222 (24%)
  pending 0/935, coverage 6.22%, no crashes yet

>>> fuzzer02 (4 days, 23 hrs) <<<

  cycle 376, lifetime speed 165 execs/sec, path 87/1220 (7%)
  pending 0/0, coverage 6.22%, no crashes yet

>>> fuzzer03 (4 days, 23 hrs) <<<

  cycle 379, lifetime speed 166 execs/sec, path 114/1211 (9%)
  pending 0/0, coverage 6.22%, no crashes yet

>>> fuzzer04 (4 days, 23 hrs) <<<

  cycle 388, lifetime speed 166 execs/sec, path 1030/1211 (85%)
  pending 0/0, coverage 6.22%, no crashes yet

>>> fuzzer05 (2 days, 4 hrs) <<<

  cycle 161, lifetime speed 162 execs/sec, path 62/1227 (5%)
  pending 0/0, coverage 6.22%, no crashes yet

Summary stats
=============

       Fuzzers alive : 5
      Total run time : 22 days, 1 hours
         Total execs : 294 million
    Cumulative speed : 773 execs/sec
       Pending paths : 0 faves, 935 total
  Pending per fuzzer : 0 faves, 187 total (on average)
       Crashes found : 0 locally unique

sipa · 2016-12-01T01:12:40Z

Concept ACK

maflcko · 2016-12-02T19:31:10Z

I think this ready for merge and we should select and add appropriate afl-inputs in another pull.

(Maybe fix the doc nits before merge?)

laanwj · 2016-12-03T06:42:46Z

I think this ready for merge and we should select and add appropriate afl-inputs in another pull.

I don't think we should put the inputs in the repository. But I'll just add a link where they can be downloaded. Will fix the doc nits.

pstratem · 2016-12-04T20:44:57Z

My current test cases are here http://strateman.ninja/fuzzing.tar.xz

maflcko · 2016-12-05T10:41:30Z

src/test/test_bitcoin_fuzzy.cpp

+            try
+            {
+                CTransaction tx;
+                ds >> tx;


error: ‘class CTransaction’ has no member named ‘Unserialize’

right, this should probably be converted to CMutableTransaction

laanwj · 2016-12-15T15:54:32Z

Switched to the new way of transaction deserialization. This should be ready for merge now.

8b15434 doc: Add bare-bones documentation for fuzzing (Wladimir J. van der Laan) a4153e2 Simple fuzzing framework (Patrick Strateman)

sipa · 2016-12-15T17:21:59Z

utACK 8b15434 (apart from #9354).

8b15434 doc: Add bare-bones documentation for fuzzing (Wladimir J. van der Laan) a4153e2 Simple fuzzing framework (Patrick Strateman)

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

laanwj added the Tests label Nov 16, 2016

laanwj added this to the 0.14.0 milestone Nov 16, 2016

fanquake reviewed Nov 23, 2016

View reviewed changes

maflcko reviewed Nov 25, 2016

View reviewed changes

laanwj force-pushed the 2016_11_fuzzing_framework branch 2 times, most recently from 0152939 to 8e0550f Compare December 5, 2016 07:46

maflcko reviewed Dec 5, 2016

View reviewed changes

pstratem and others added 2 commits December 15, 2016 13:29

Simple fuzzing framework

a4153e2

doc: Add bare-bones documentation for fuzzing

8b15434

laanwj force-pushed the 2016_11_fuzzing_framework branch from 8e0550f to 8b15434 Compare December 15, 2016 12:29

laanwj merged commit 8b15434 into bitcoin:master Dec 15, 2016

laanwj added a commit that referenced this pull request Dec 15, 2016

Merge #9172: Resurrect pstratem's "Simple fuzzing framework"

5bc209c

8b15434 doc: Add bare-bones documentation for fuzzing (Wladimir J. van der Laan) a4153e2 Simple fuzzing framework (Patrick Strateman)

sipa mentioned this pull request Dec 15, 2016

Make fuzzer actually test CTxOutCompressor #9354

Merged

laanwj mentioned this pull request Dec 16, 2016

fundrawtransaction.py test broken in travis ("Mempool sync failed") #9364

Closed

furszy mentioned this pull request Mar 17, 2021

Fuzzing framework support PIVX-Project/PIVX#2252

Merged

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

Resurrect pstratem's "Simple fuzzing framework" #9172

Resurrect pstratem's "Simple fuzzing framework" #9172

Conversation

laanwj commented Nov 16, 2016 • edited

jonasschnelli commented Nov 16, 2016

laanwj commented Nov 16, 2016

laanwj commented Nov 16, 2016

maflcko commented Nov 16, 2016

laanwj commented Nov 16, 2016

maflcko commented Nov 16, 2016

laanwj commented Nov 16, 2016

pstratem commented Nov 19, 2016 • edited

laanwj commented Nov 19, 2016 via email

gmaxwell commented Nov 19, 2016

laanwj commented Nov 21, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fanquake commented Nov 24, 2016 • edited

fanquake commented Nov 24, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maflcko commented Nov 25, 2016

Choose a reason for hiding this comment

fanquake Nov 25, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maflcko Nov 25, 2016 • edited

Choose a reason for hiding this comment

laanwj commented Nov 25, 2016 via email

fanquake commented Nov 25, 2016 • edited

theuni commented Nov 25, 2016

laanwj commented Nov 26, 2016 via email

pstratem commented Nov 27, 2016 • edited

fanquake commented Nov 27, 2016 • edited

fanquake commented Nov 29, 2016

sipa commented Dec 1, 2016

maflcko commented Dec 2, 2016

laanwj commented Dec 3, 2016

pstratem commented Dec 4, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laanwj commented Dec 15, 2016

sipa commented Dec 15, 2016

laanwj commented Nov 16, 2016 •

edited

pstratem commented Nov 19, 2016 •

edited

laanwj commented Nov 21, 2016 •

edited

fanquake commented Nov 24, 2016 •

edited

fanquake commented Nov 24, 2016 •

edited

fanquake Nov 25, 2016 •

edited

maflcko Nov 25, 2016 •

edited

fanquake commented Nov 25, 2016 •

edited

pstratem commented Nov 27, 2016 •

edited

fanquake commented Nov 27, 2016 •

edited