Fuzzing framework support #2252

furszy · 2021-03-17T03:12:25Z

As the title says, adding fuzzing framework support so we can start getting serious on this area as well.

Adapted the following PRs:

furszy · 2021-03-24T15:20:20Z

Short update,
Went further down upstream's path, improved the fuzzing.md document, disabled other unneeded targets when fuzz is enabled, added a python test_runner to run the fuzzer simpler test/fuzz/test_runner.py (testing needed..), added fuzzing harness for CheckTransaction(), IsStandard() and several CTransaction methods, plus added new cases to the deserialization targets.

As the changes are pretty isolated from everything else, only touching the new fuzzing framework, this PR shouldn't be too hard to review even when it has a good number of commits.

random-zebra

This is looking awesome. Been testing with AFL for a while and all is good.
Still need to check libfuzzer and the python script.

Some initial feedback while going through the code: needs rebase on master (and slight rework) for the changes to CheckTransaction, and the documentation could use a bit of "pivx-fication".

doc/developer-notes.md

doc/fuzzing.md

src/test/fuzz/transaction.cpp

random-zebra · 2021-04-17T20:06:57Z

.travis/test_06_script_b.sh

Since we are no longer using Travis, it would be awesome to have the same thing for GA in the future.

furszy · 2021-04-20T21:28:24Z

Rebased on master + updated per feedback.

Updated CheckTransaction function call to the new function signature.
Added ContextualCheckTransaction function call to the transaction fuzz target.
Fuzzing documentation "PIVXified".

furszy · 2021-05-04T14:17:34Z

Rebased on master so this work does not get behind.

random-zebra

ACK 0d3af08cc24c9980862d95c9fd4ec4caeec01029 (didn't complete all the inputs... it's going too slow here).

Great job 🥃
I think this can be merged already, as it is a completely separated module, not touching the core code directly. Other upgrades can be done in successive pull requests.

Fuzzbawls · 2021-05-09T00:19:19Z

I'm getting a linker error when building with --enable-fuzz using gcc, afl, and clang. Changing the link order as follows fixes it up for me on linux, but haven't tested macOS:

Index: src/Makefile.test.include
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/Makefile.test.include b/src/Makefile.test.include
--- a/src/Makefile.test.include	(revision 5f0ae0f7f4863ec6c75f8124b5331403003df9b3)
+++ b/src/Makefile.test.include	(date 1620471251595)
@@ -198,7 +198,7 @@
 if ENABLE_ZMQ
 test_test_pivx_fuzzy_LDADD += $(LIBBITCOIN_ZMQ) $(ZMQ_LIBS)
 endif
-test_test_pivx_fuzzy_LDADD += $(LIBBITCOIN_SERVER) $(LIBBITCOIN_CLI) $(LIBBITCOIN_COMMON) $(LIBBITCOIN_UTIL) $(LIBUNIVALUE) $(LIBBITCOIN_ZEROCOIN) \
+test_test_pivx_fuzzy_LDADD += $(LIBBITCOIN_SERVER) $(LIBBITCOIN_CLI) $(LIBBITCOIN_COMMON) $(LIBBITCOIN_ZEROCOIN) $(LIBBITCOIN_UTIL) $(LIBUNIVALUE) \
   $(LIBLEVELDB) $(LIBLEVELDB_SSE42) $(LIBMEMENV) $(BOOST_LIBS) $(LIBSECP256K1) $(EVENT_LIBS) $(EVENT_PTHREADS_LIBS)
 if ENABLE_WALLET
 test_test_pivx_fuzzy_LDADD += $(LIBBITCOIN_WALLET)

Other than this, I agree it could be merged sooner rather than later. Also, do we want to eventually maintain our own clone of the qa-assets repo? as surely our inputs will diverge from upstream.

This avoids calling things like pubkey_parse with a null context argument.

Enable the `afl-clang-fast++` features deferred forkserver (`__AFL_INIT`) and persistent mode (`__AFL_LOOP(1000)`). Before this patch: ``` $ afl-fuzz -i input -o output -m512 -- src/test/test_bitcoin_fuzzy [*] Validating target binary... [!] WARNING: The target binary is pretty slow! See /usr/local/share/doc/afl/perf_tips.txt. [+] Here are some useful stats: Test case count : 1 favored, 0 variable, 1 total Bitmap range : 1072 to 1072 bits (average: 1072.00 bits) Exec timing : 20.4k to 20.4k us (average: 20.4k us) … exec speed : 57.58/sec (slow!) exec speed : 48.35/sec (slow!) exec speed : 53.78/sec (slow!) ``` After this patch: ``` $ afl-fuzz -i input -o output -m512 -- src/test/test_bitcoin_fuzzy [*] Validating target binary... [+] Persistent mode binary detected. [+] Deferred forkserver binary detected. [+] Here are some useful stats: Test case count : 1 favored, 0 variable, 1 total Bitmap range : 24 to 24 bits (average: 24.00 bits) Exec timing : 114 to 114 us (average: 114 us) … exec speed : 15.9k/sec exec speed : 13.1k/sec exec speed : 15.1k/sec ```

See http://llvm.org/docs/LibFuzzer.html#fuzzer-usage for usage instructions.

This is a crash cause, only affecting us because we use the param for data deserialization like the block index.

Coming from btc@fab2daa026494cdacda530f1253eb43cf0f1576c

The coverage statistics are not stable across clang versions

….) and other CTransaction related functions Based on btc@0a573682f24d20cb178b8b6f97c35ec46901c4db with PIVX changes on top.

adaptation of btc@897849d8c225045f0dd3a2fe99b5d69bdf84b4e2 without the objects that we don't have.

…und-trip equality where possible. Avoid code repetition.

Coming straight from btc@6feb46c3721605f6ae54d7e635a06f82cd7c1449

furszy · 2021-05-26T15:28:23Z

Hmm that is weird, i have been able to compile the fuzz target in macOS and linux (clang and gcc) fine all of this time and zebra too. But can add the link order change, compiled fine with it as well.

About the qa-assets repo, yeah. We differ from upstream seed inputs already due the fact that we don't share the same data model. The serialization/deserialization and validation are different (transactions, block headers, block signature, inventory msgs include tier two types etc).

i'm planning to push the new repo when we finish #2267 work list, the modifications and news included there make some of the current inputs invalid, which will force us to restart the entire fuzzing process to generate the correct inputs (which is so so slow and cpu consuming that is better to do it once the new changes are settled).

furszy · 2021-05-26T15:37:35Z

Rebased on master + included the libzerocoin link order change. Ready to go.

random-zebra

utACK d059544 and merging...

furszy self-assigned this Mar 17, 2021

furszy force-pushed the 2020_fuzzing_framework branch 2 times, most recently from ace0c21 to eb7c315 Compare March 17, 2021 20:28

furszy changed the title ~~[WIP] Initial fuzzing framework support~~ Initial fuzzing framework support Mar 17, 2021

furszy changed the title ~~Initial fuzzing framework support~~ [WIP] Initial fuzzing framework support Mar 17, 2021

furszy force-pushed the 2020_fuzzing_framework branch 2 times, most recently from 850425b to c6cae40 Compare March 22, 2021 19:08

furszy changed the title ~~[WIP] Initial fuzzing framework support~~ [WIP] Fuzzing framework support Mar 22, 2021

furszy force-pushed the 2020_fuzzing_framework branch from 433dac7 to 9f54131 Compare March 23, 2021 00:00

random-zebra added this to the 6.0.0 milestone Mar 25, 2021

furszy force-pushed the 2020_fuzzing_framework branch from 0612b76 to 94eb714 Compare March 26, 2021 23:19

furszy force-pushed the 2020_fuzzing_framework branch from 94eb714 to a5f23cd Compare April 8, 2021 18:01

furszy changed the title ~~[WIP] Fuzzing framework support~~ Fuzzing framework support Apr 16, 2021

random-zebra reviewed Apr 17, 2021

View reviewed changes

random-zebra added Tests Upstream labels Apr 17, 2021

furszy force-pushed the 2020_fuzzing_framework branch from a5f23cd to 0df5cec Compare April 20, 2021 16:29

furszy force-pushed the 2020_fuzzing_framework branch from 31329ee to 0d3af08 Compare May 4, 2021 14:15

random-zebra previously approved these changes May 6, 2021

View reviewed changes

pstratem and others added 7 commits May 26, 2021 12:17

Simple fuzzing framework

5c3b550

doc: Add bare-bones documentation for fuzzing

d6f6a85

Make fuzzer actually test CTxOutCompressor

11150df

Init ECC context for test_bitcoin_fuzzy.

faf2be6

This avoids calling things like pubkey_parse with a null context argument.

[tests] Add libFuzzer support.

08d8ebe

See http://llvm.org/docs/LibFuzzer.html#fuzzer-usage for usage instructions.

[fuzzing] initialize chain params by default.

2e4ec58

This is a crash cause, only affecting us because we use the param for data deserialization like the block index.

furszy and others added 22 commits May 26, 2021 12:17

Add missing LIBBITCOIN_ZMQ to test target

89fe5b2

Coming from btc@fab2daa026494cdacda530f1253eb43cf0f1576c

qa: Add test/fuzz/test_runner.py

541f442

fuzz: test_runner: Better error message when built with afl

425742c

build: Allow to configure --with-sanitizers=fuzzer

f28ac9a

Disable other targets when enable-fuzz is set

1266d3e

Update doc and CI config

c3447b5

[Build] Do not disable wallet when fuzz is enabled.

d642b67

doc: Improve fuzzing docs for macOS users

48cd0c8

test: Log output even if fuzzer failed

cd6134f

tests: Improve test runner output in case of target errors

b54b1d6

doc: Remove --disable-ccache from docs

2b4f8aa

fuzz: Add option to merge input dir to test runner

52693ee

fuzz: Remove option --export_coverage from test_runner

3205871

The coverage statistics are not stable across clang versions

tests: Add fuzzing harness for CheckTransaction(...), IsStandardTx(..…

b5f291c

….) and other CTransaction related functions Based on btc@0a573682f24d20cb178b8b6f97c35ec46901c4db with PIVX changes on top.

tests: Remove TRANSACTION_DESERIALIZE (replaced by transaction fuzzer)

e1f666c

tests: Add deserialization fuzzing harnesses

d058d8c

adaptation of btc@897849d8c225045f0dd3a2fe99b5d69bdf84b4e2 without the objects that we don't have.

ignore new fuzz targets gitignore

e1b92b6

tests: Test serialisation as part of deserialisation fuzzing. Test ro…

70a0ace

…und-trip equality where possible. Avoid code repetition.

[doc] add sanitizers documentation in developer-notes.md

9631f46

Coming straight from btc@6feb46c3721605f6ae54d7e635a06f82cd7c1449

Fuzzing documentation "PIVX-fication"

f0887a0

[fuzz] Add ContextualCheckTransaction call to transaction target.

2396e6b

[Build] fuzz target, change LIBBITCOIN_ZEROCOIN link order.

d059544

furszy dismissed random-zebra’s stale review via d059544 May 26, 2021 15:36

furszy force-pushed the 2020_fuzzing_framework branch from 0d3af08 to d059544 Compare May 26, 2021 15:36

furszy requested a review from random-zebra May 27, 2021 18:41

random-zebra approved these changes May 28, 2021

View reviewed changes

random-zebra merged commit 44b5327 into PIVX-Project:master May 28, 2021

furszy deleted the 2020_fuzzing_framework branch November 29, 2022 14:21

Fuzzing framework support #2252

Fuzzing framework support #2252

Uh oh!

Conversation

furszy commented Mar 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

furszy commented Mar 24, 2021

Uh oh!

random-zebra left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

random-zebra Apr 17, 2021

Choose a reason for hiding this comment

Uh oh!

furszy commented Apr 20, 2021

Uh oh!

furszy commented May 4, 2021

Uh oh!

random-zebra left a comment

Choose a reason for hiding this comment

Uh oh!

Fuzzbawls commented May 9, 2021

Uh oh!

furszy commented May 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

furszy commented May 26, 2021

Uh oh!

random-zebra left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

furszy commented Mar 17, 2021 •

edited

Loading

furszy commented May 26, 2021 •

edited

Loading