Skip to content

tests: Add fuzzing harness for CheckBlock(...) and other CBlock related functions - #17071

Merged
maflcko merged 2 commits into
bitcoin:masterfrom
practicalswift:fuzzers-block
Dec 16, 2019
Merged

tests: Add fuzzing harness for CheckBlock(...) and other CBlock related functions#17071
maflcko merged 2 commits into
bitcoin:masterfrom
practicalswift:fuzzers-block

Conversation

@practicalswift

@practicalswift practicalswift commented Oct 7, 2019

Copy link
Copy Markdown
Contributor

Add fuzzing harness for CheckBlock(...) and other CBlock related functions.

Testing this PR

Run:

$ CC=clang CXX=clang++ ./configure --enable-fuzz --with-sanitizers=address,fuzzer,undefined
$ make
$ src/test/fuzz/block
…
# And to to quickly verify that the relevant code regions are triggered, that the
# fuzzing throughput seems reasonable, etc.
$ contrib/devtools/test_fuzzing_harnesses.sh '^block$'

test_fuzzing_harnesses.sh can be found in PR #17000.

Comment thread src/test/fuzz/fuzz.cpp Outdated

@maflcko maflcko left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Concept ACK

Comment thread src/test/fuzz/block.cpp Outdated
Comment thread src/test/fuzz/block.cpp Outdated
@DrahtBot

DrahtBot commented Oct 7, 2019

Copy link
Copy Markdown
Contributor

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #17229 (tests: Add fuzzing harnesses for various Base{32,58,64} and hex related functions 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.

@practicalswift

Copy link
Copy Markdown
Contributor Author

Rebased! :)

@practicalswift

Copy link
Copy Markdown
Contributor Author

Rebased and added a fuzzing harness for ContextualCheckBlock(…).

@practicalswift

practicalswift commented Oct 31, 2019

Copy link
Copy Markdown
Contributor Author

@MarcoFalke

When fuzzing BlockWitnessMerkleRoot(…) I noticed that leaves[0].SetNull() is done also in the case when leaves.empty() (due to block.vtx.empty()):

uint256 BlockWitnessMerkleRoot(const CBlock& block, bool* mutated)
{
std::vector<uint256> leaves;
leaves.resize(block.vtx.size());
leaves[0].SetNull(); // The witness hash of the coinbase is 0.
for (size_t s = 1; s < block.vtx.size(); s++) {
leaves[s] = block.vtx[s]->GetWitnessHash();
}
return ComputeMerkleRoot(std::move(leaves), mutated);
}

Makes sense to guard against this? :)

Update: Working around this by simply skipping the call to BlockWitnessMerkleRoot(…) in case of block.vtx.empty().

@practicalswift

Copy link
Copy Markdown
Contributor Author

Rebased! :)

@practicalswift

Copy link
Copy Markdown
Contributor Author

Should be ready for final review :)

Comment thread src/test/fuzz/block.cpp Outdated
@maflcko

maflcko commented Dec 13, 2019

Copy link
Copy Markdown
Member

ACK c5d08da5c38e4140f4b9465f3e8159bcd7874ab2

Comment thread src/test/fuzz/block.cpp Outdated
maflcko pushed a commit that referenced this pull request Dec 16, 2019
…r CBlock related functions

893aa20 tests: Add fuzzing harness for CheckBlock(...) and other CBlock related functions (practicalswift)
ec8dcb0 tests: Add corpora suppression (FUZZERS_MISSING_CORPORA) for fuzzers missing in https://github.com/bitcoin-core/qa-assets/tree/master/fuzz_seed_corpus (practicalswift)

Pull request description:

  Add fuzzing harness for `CheckBlock(...)` and other `CBlock` related functions.

  **Testing this PR**

  Run:

  ```
  $ CC=clang CXX=clang++ ./configure --enable-fuzz --with-sanitizers=address,fuzzer,undefined
  $ make
  $ src/test/fuzz/block
  …
  # And to to quickly verify that the relevant code regions are triggered, that the
  # fuzzing throughput seems reasonable, etc.
  $ contrib/devtools/test_fuzzing_harnesses.sh '^block$'
  ```

  `test_fuzzing_harnesses.sh` can be found in PR #17000.

Top commit has no ACKs.

Tree-SHA512: 275abd46d8ac970b28d8176f59124988b1e07c070173e001acd55995b830333417f301c309199fc589da08a6ac4c03aa74650d5e1638f6e3023dfbd3c9f6921d
@maflcko
maflcko merged commit 893aa20 into bitcoin:master Dec 16, 2019
jasonbcox pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Oct 28, 2020
…ed functions

Summary:
```
Add fuzzing harness for CheckBlock(...) and other CBlock related
functions.
```

Backport of core [[bitcoin/bitcoin#17071 | PR17071]].

The missing corpora change is not relevant for us.

Test Plan:
  ninja bitcoin-fuzzers
  ./test/fuzz/test_runner.py <path_to_corpus>

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D8151
@practicalswift
practicalswift deleted the fuzzers-block branch April 10, 2021 19:39
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Mar 7, 2022
…nd other CBlock related functions

893aa20 tests: Add fuzzing harness for CheckBlock(...) and other CBlock related functions (practicalswift)
ec8dcb0 tests: Add corpora suppression (FUZZERS_MISSING_CORPORA) for fuzzers missing in https://github.com/bitcoin-core/qa-assets/tree/master/fuzz_seed_corpus (practicalswift)

Pull request description:

  Add fuzzing harness for `CheckBlock(...)` and other `CBlock` related functions.

  **Testing this PR**

  Run:

  ```
  $ CC=clang CXX=clang++ ./configure --enable-fuzz --with-sanitizers=address,fuzzer,undefined
  $ make
  $ src/test/fuzz/block
  …
  # And to to quickly verify that the relevant code regions are triggered, that the
  # fuzzing throughput seems reasonable, etc.
  $ contrib/devtools/test_fuzzing_harnesses.sh '^block$'
  ```

  `test_fuzzing_harnesses.sh` can be found in PR bitcoin#17000.

Top commit has no ACKs.

Tree-SHA512: 275abd46d8ac970b28d8176f59124988b1e07c070173e001acd55995b830333417f301c309199fc589da08a6ac4c03aa74650d5e1638f6e3023dfbd3c9f6921d
PastaPastaPasta added a commit to dashpay/dash that referenced this pull request Mar 29, 2022
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 16, 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.

4 participants