Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: Add fuzzing harness for ProcessMessage(...). Enables high-level fuzzing of the P2P layer. #17989

Merged
merged 2 commits into from Mar 11, 2020

Conversation

practicalswift
Copy link
Contributor

Add fuzzing harness for ProcessMessage(...). Enables high-level fuzzing of the P2P layer.

All code paths reachable from this fuzzer can be assumed to be reachable for an untrusted peer.

Seeded from thin air (an empty corpus) this fuzzer reaches roughly 20 000 lines of code.

To test this PR:

$ make distclean
$ ./autogen.sh
$ CC=clang CXX=clang++ ./configure --enable-fuzz \
      --with-sanitizers=address,fuzzer,undefined
$ make
$ src/test/fuzz/process_message
…

Worth noting about this fuzzing harness:

  • To achieve a reasonable number of executions per seconds the state of the fuzzer is unfortunately not entirely reset between test_one_input calls. The set-up (FuzzingSetup ctor) and tear-down (~FuzzingSetup) work is simply too costly to be run on every iteration. There is a trade-off to handle here between a.) achieving high executions/second and b.) giving the fuzzer a totally blank slate for each call. Please let me know if you have any suggestion on how to improve this situation while maintaining >1000 executions/second.
  • To achieve optimal results when using coverage-guided fuzzing I've chosen to create one specialised fuzzing binary per message type (process_message_addr, process_message_block, process_message_blocktxn , etc.) and one general fuzzing binary (process_message) which handles all messages types. The latter general fuzzer can be seeded with inputs generated by the former specialised fuzzers.

Happy fuzzing friends!

@DrahtBot
Copy link
Contributor

DrahtBot commented Jan 23, 2020

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

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
Copy link
Member

laanwj commented Feb 10, 2020

code review ACK 1b67435

@practicalswift
Copy link
Contributor Author

@laanwj Thanks for reviewing. Pushed a commit which removes the tinyformat dependency from ToString(…). Please re-review :)

@practicalswift
Copy link
Contributor Author

Rebased! :)

@practicalswift
Copy link
Contributor Author

Rebased :)

Copy link
Member

@maflcko maflcko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concept ACK. Is there a reason to both allow all message types and then add some fuzzers that only allow one message type?

src/test/fuzz/process_message.cpp Outdated Show resolved Hide resolved
src/test/fuzz/process_message.cpp Outdated Show resolved Hide resolved
@practicalswift practicalswift force-pushed the fuzzers-net-process_message branch 3 times, most recently from 10ee74e to 3e2185c Compare March 10, 2020 18:23
Copy link
Member

@maflcko maflcko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 4d4f38e 🔒

Show signature and timestamp

Signature:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK 4d4f38ee5a 🔒
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUjV/gwAyGMYagoM2OCSoUvGaS0o3rBEIIUJ6lt8R/i91LUEuc46NitcJk/GH61o
fH0lnYi3lmLxRxk2BUvwxY7aS+0lHExqcz/O2VBd7A7A8iqQ4TVUcMibXcJMdNrO
C3b+8ndKv18XuYvSaoSWOAG1E9smnB824klvz6gQoF45Ku6qgAd1b/05D85lQf1D
8t5ovpvjN6et1jWoRR5VTL6UeUdtDcwM/HatJ/S6hHqDqbtYlV0HWhqXwt+DqmvY
6Tl6w2yKRXrvrAaLwSFmD+iEQOYSkiEGJgxchjGuy6lo4rQPUQBCwKSZtOEGobLO
qlC1iVhjKHWVflitLkC6u1oW/4domq/ahqkt+PMg9uJMMsMt9L1UtC1t9gqqC7qk
LDAQihxRnYF5jWN5Is25/Qp5OsVRQU8VZQAQ8EpxBHVWkHWIloT+lvXVSp0xbNyJ
KnwQtKxpZ+cSGqd/8nX4aLwTMXo+v5H9+5DudrZ4gpQLdZKp4RqMOyrwWRTdsCfn
vdu0tFx9
=aujQ
-----END PGP SIGNATURE-----

Timestamp of file with hash 730802cd0481d1a196758386a01e5111c8d68b5c12c7079e969bf05a0cec0a02 -

src/test/fuzz/process_message.cpp Outdated Show resolved Hide resolved
src/test/fuzz/process_message.cpp Outdated Show resolved Hide resolved
src/test/fuzz/process_message.cpp Outdated Show resolved Hide resolved
src/test/fuzz/process_message.cpp Outdated Show resolved Hide resolved
src/test/fuzz/process_message.cpp Outdated Show resolved Hide resolved
src/test/fuzz/process_message.cpp Outdated Show resolved Hide resolved
src/test/util/setup_common.cpp Outdated Show resolved Hide resolved
src/test/fuzz/process_message.cpp Outdated Show resolved Hide resolved
src/test/fuzz/process_message.cpp Outdated Show resolved Hide resolved
src/test/fuzz/process_message.cpp Outdated Show resolved Hide resolved
@maflcko
Copy link
Member

maflcko commented Mar 10, 2020

Also, travis is failing

@practicalswift
Copy link
Contributor Author

Concept ACK. Is there a reason to both allow all message types and then add some fuzzers that only allow one message type?

Yes there is :)

  1. The reason to allow all message types in one of the fuzzers: this allows auto-detection and thus fuzzing of newly introduced messages types without updating the fuzzer. See Erlay: bandwidth-efficient transaction relay protocol #18261 (comment) for a real-world example of that :)

  2. The reason to have also have per-message-type fuzzers is largely for the same reason that we have one fuzzing binary per deserialization: to make it relatively easier for coverage guided fuzzers to reach deep. See sipa's rationale here and kcc's confirmation here. Note that @kcc is the one of the libFuzzer authors :)

@practicalswift practicalswift force-pushed the fuzzers-net-process_message branch 4 times, most recently from 3076814 to 626174d Compare March 10, 2020 20:22
@naumenkogs
Copy link
Member

Concept ACK. This seems super-useful.

@maflcko
Copy link
Member

maflcko commented Mar 11, 2020

ACK 9220a0f 🏊

Show signature and timestamp

Signature:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK 9220a0fdd0 🏊
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUjT1QwAhjXDNbw5aYFTlg68glVfl5CPdMNZ4gKbD/RkTp3bWXzxo7f/RvRh1WCc
+ifotMLe4Ihdj2r6H6guhYy46Damnk+qPhaDfLxGQAc2mwZrJ4gHiS2LDiZsvckD
X6+SMgJK2uHEy9u5zjXLEFOoHArGb2J73XloMTmJR7HTLlFy53hEMJsYqu9+Fmo+
sxzXBF1Mn40I+NnHr9bEPe66fgw6Nf3PiM9CIbJLQRBhCXRLAnkurO6Ai7gK71a+
DjxaCie+KBcpupVdbHPTRIyRO98jSndZYzK9ksSDshh2ifxGhrUQGSKRQ3e6CGtZ
fgZsB3o+TKmMPKIawyxU1oAfpuCmWC/seqoLPvR7ymoYgd+gsNZGjp7w+hVbiku9
6SwRrQSRqGg5cio4qWbqqlyqmzGBpDrs9MjcY3/NF/mJrafarZ2UB+BKQ5IrXkZj
BidwPFy4zJS+8hPUGeRqH8igHzG3cY5xkB1Q+Wpy2McjDguh1qkzWJcU/9ifRqnj
2DQxCmcw
=VmPE
-----END PGP SIGNATURE-----

Timestamp of file with hash 11743d3aff704a992c2abdf56c51b9f75f57199d2d9b269edff015d74f3fe9c3 -

@maflcko maflcko merged commit f1064c1 into bitcoin:master Mar 11, 2020
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Oct 20, 2020
…l fuzzing of the P2P layer.

Summary:
```
Add fuzzing harness for ProcessMessage(...). Enables high-level fuzzing
of the P2P layer.

All code paths reachable from this fuzzer can be assumed to be reachable
for an untrusted peer.

Seeded from thin air (an empty corpus) this fuzzer reaches roughly 20
000 lines of code.

To test this PR:

$ make distclean
$ ./autogen.sh
$ CC=clang CXX=clang++ ./configure --enable-fuzz \
      --with-sanitizers=address,fuzzer,undefined
$ make
$ src/test/fuzz/process_message
…

Worth noting about this fuzzing harness:

    To achieve a reasonable number of executions per seconds the state
of the fuzzer is unfortunately not entirely reset between test_one_input
calls. The set-up (FuzzingSetup ctor) and tear-down (~FuzzingSetup) work
is simply too costly to be run on every iteration. There is a trade-off
to handle here between a.) achieving high executions/second and b.)
giving the fuzzer a totally blank slate for each call. Please let me
know if you have any suggestion on how to improve this situation while
maintaining >1000 executions/second.
    To achieve optimal results when using coverage-guided fuzzing I've
chosen to create one specialised fuzzing binary per message type
(process_message_addr, process_message_block, process_message_blocktxn ,
etc.) and one general fuzzing binary (process_message) which handles all
messages types. The latter general fuzzer can be seeded with inputs
generated by the former specialised fuzzers.

Happy fuzzing friends!
```

Backport od core [[bitcoin/bitcoin#17989 | PR17989]].

Depends on D8004 (test plan only, fixes a fuzz fixture issue).

Test Plan:
  ninja bitcoin-fuzzers
  ./src/test/fuzz/process_message
  ./src/test/fuzz/process_message_getheaders # Or any other message

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Subscribers: deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D8005
@practicalswift practicalswift deleted the fuzzers-net-process_message branch April 10, 2021 19:40
kwvg added a commit to kwvg/dash that referenced this pull request Jan 9, 2022
…nables high-level fuzzing of the P2P layer
kwvg added a commit to kwvg/dash that referenced this pull request Jan 11, 2022
…nables high-level fuzzing of the P2P layer
kwvg added a commit to kwvg/dash that referenced this pull request Jan 25, 2022
…nables high-level fuzzing of the P2P layer
kwvg added a commit to kwvg/dash that referenced this pull request Feb 19, 2022
…nables high-level fuzzing of the P2P layer
kwvg added a commit to kwvg/dash that referenced this pull request Mar 25, 2022
…nables high-level fuzzing of the P2P layer
kwvg added a commit to kwvg/dash that referenced this pull request Mar 30, 2022
…nables high-level fuzzing of the P2P layer
kwvg added a commit to kwvg/dash that referenced this pull request Apr 3, 2022
…nables high-level fuzzing of the P2P layer
kwvg added a commit to kwvg/dash that referenced this pull request Apr 3, 2022
…nables high-level fuzzing of the P2P layer
kwvg added a commit to kwvg/dash that referenced this pull request Apr 5, 2022
…nables high-level fuzzing of the P2P layer

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
kwvg added a commit to kwvg/dash that referenced this pull request Apr 5, 2022
…nables high-level fuzzing of the P2P layer

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
kwvg added a commit to kwvg/dash that referenced this pull request Apr 6, 2022
…nables high-level fuzzing of the P2P layer

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
kwvg added a commit to kwvg/dash that referenced this pull request Apr 6, 2022
…nables high-level fuzzing of the P2P layer

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
kwvg added a commit to kwvg/dash that referenced this pull request Apr 6, 2022
…nables high-level fuzzing of the P2P layer

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
kwvg added a commit to kwvg/dash that referenced this pull request Apr 7, 2022
…nables high-level fuzzing of the P2P layer

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
kwvg added a commit to kwvg/dash that referenced this pull request Apr 7, 2022
…nables high-level fuzzing of the P2P layer

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
kwvg added a commit to kwvg/dash that referenced this pull request Apr 17, 2022
…nables high-level fuzzing of the P2P layer

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
kwvg added a commit to kwvg/dash that referenced this pull request Apr 19, 2022
…nables high-level fuzzing of the P2P layer

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
PastaPastaPasta added a commit to dashpay/dash that referenced this pull request Apr 20, 2022
kwvg added a commit to kwvg/dash that referenced this pull request Jul 6, 2022
…binary per message type for optimal results when using coverage-guided fuzzing
kwvg added a commit to kwvg/dash that referenced this pull request Jul 6, 2022
…binary per message type for optimal results when using coverage-guided fuzzing
kwvg added a commit to kwvg/dash that referenced this pull request Jul 13, 2022
…binary per message type for optimal results when using coverage-guided fuzzing
kwvg added a commit to kwvg/dash that referenced this pull request Jul 13, 2022
…binary per message type for optimal results when using coverage-guided fuzzing
kwvg added a commit to kwvg/dash that referenced this pull request Jul 15, 2022
…binary per message type for optimal results when using coverage-guided fuzzing
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants