Page MenuHomePhabricator

[backports] tests: Skip unnecessary fuzzer initialisation. Hold ECCVerifyHandle only when needed.
ClosedPublic

Authored by majcosta on Jul 9 2020, 21:48.

Details

Summary

This diff squashes three Core PRs into one. The reason is that PR17235 introduces a bug, and PR17274 and PR17685 both fix it, so our fuzzing test setup isn't broken at any point.


c2f964a6745be085f2891c909d6c998687de9080 tests: Remove Cygwin WinMain workaround (practicalswift)
db4bd32cc31789fc017f5db0b86a69ee43e41575 tests: Skip unnecessary fuzzer initialisation. Hold ECCVerifyHandle only when needed. (practicalswift)

Pull request description:

Skip unnecessary fuzzer initialisation. Hold `ECCVerifyHandle` only when needed.

As suggested by MarcoFalke in https://github.com/bitcoin/bitcoin/pull/17018#discussion_r336645391.

Merge #17274: tests: Fix fuzzers eval_script and script_flags by re-adding ECCVerifyHandle dependency

9cae3d5e94f4481e0d251c924314e57187a07a60 tests: Add fuzzer initialization (hold ECCVerifyHandle) (practicalswift)

Pull request description:

The fuzzers `eval_script` and `script_flags` require holding `ECCVerifyHandle`.

This is a follow-up to #17235 which accidentally broke those two fuzzers.

Sorry about the temporary breakage my fuzzing friends: it took a while to fuzz before reaching these code paths. That's why this wasn't immediately caught. Sorry.

Merge #17685: tests: Fix bug in the descriptor parsing fuzzing harness (descriptor_parse)

6338c0203416a5f86e9422b6cd479da8af277f2f tests: Fix fuzzing harness for descriptor parsing (descriptor_parse) (practicalswift)

Pull request description:

Fix bug in the descriptor parsing fuzzing harness (`descriptor_parse`) by making sure `secp256k1_context_verify` is properly initialized (via `ECCVerifyHandle`).

Background:

When fuzzing `Parse(…)` with `libFuzzer` I eventually reached the test case `combo(020000000000000000000000000000000000000000000000000000000000000000)`. That input triggers a call to `CPubKey::IsFullyValid()` which in turns requires an initialized `secp256k1_context_verify`.

The fuzzing harness did not fulfil that pre-condition prior to this commit (sorry, my fault!) :)

Depends on D6881

Backport of Core PR17235, PR17274 and PR17685

Test Plan
cmake -GNinja .. -DENABLE_SANITIZERS="address;fuzzer" -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++
ninja bitcoin-fuzzers link-fuzz-test_runner.py
./test/fuzz/test-runner.py -l DEBUG <path-to-corpus>

Event Timeline

majcosta requested review of this revision.Jul 9 2020, 21:48

[Bot Message]
One or more PR numbers were detected in the summary.
Links to those PRs have been inserted into the summary for reference.

This revision is now accepted and ready to land.Jul 10 2020, 06:53