Skip to content

Commit

Permalink
[backports] tests: Skip unnecessary fuzzer initialisation. Hold ECCVe…
Browse files Browse the repository at this point in the history
…rifyHandle only when needed.

Summary:
This diff squashes three Core PRs into one. The reason is that [[bitcoin/bitcoin#17235 | PR17235]] introduces a bug, and [[bitcoin/bitcoin#17274 | PR17274]] and [[bitcoin/bitcoin#17685 | 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 bitcoin/bitcoin#17018 (comment).

---

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 [[bitcoin/bitcoin#17235 | PR17235]], [[bitcoin/bitcoin#17274 | PR17274]] and [[bitcoin/bitcoin#17685 | 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>

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D6883
  • Loading branch information
MarcoFalke authored and majcosta committed Jul 10, 2020
1 parent 18edda4 commit fed8c74
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 11 deletions.
4 changes: 4 additions & 0 deletions src/test/fuzz/descriptor_parse.cpp
Expand Up @@ -3,10 +3,14 @@
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include <chainparams.h>
#include <pubkey.h>
#include <script/descriptor.h>
#include <test/fuzz/fuzz.h>

#include <memory>

void initialize() {
static const auto verify_handle = std::make_unique<ECCVerifyHandle>();
SelectParams(CBaseChainParams::REGTEST);
}

Expand Down
6 changes: 6 additions & 0 deletions src/test/fuzz/deserialize.cpp
Expand Up @@ -12,6 +12,7 @@
#include <net.h>
#include <primitives/block.h>
#include <protocol.h>
#include <pubkey.h>
#include <streams.h>
#include <undo.h>
#include <version.h>
Expand All @@ -22,6 +23,11 @@
#include <unistd.h>
#include <vector>

void initialize() {
// Fuzzers using pubkey must hold an ECCVerifyHandle.
static const auto verify_handle = std::make_unique<ECCVerifyHandle>();
}

void test_one_input(const std::vector<uint8_t> &buffer) {
CDataStream ds(buffer, SER_NETWORK, INIT_PROTO_VERSION);
try {
Expand Down
6 changes: 6 additions & 0 deletions src/test/fuzz/eval_script.cpp
Expand Up @@ -2,11 +2,17 @@
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include <pubkey.h>
#include <script/interpreter.h>
#include <test/fuzz/FuzzedDataProvider.h>
#include <test/fuzz/fuzz.h>

#include <limits>
#include <memory>

void initialize() {
static const auto verify_handle = std::make_unique<ECCVerifyHandle>();
}

void test_one_input(const std::vector<uint8_t> &buffer) {
FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
Expand Down
15 changes: 4 additions & 11 deletions src/test/fuzz/fuzz.cpp
Expand Up @@ -4,10 +4,9 @@

#include <test/fuzz/fuzz.h>

#include <pubkey.h>

#include <memory>
#include <cstdint>
#include <unistd.h>
#include <vector>

static bool read_stdin(std::vector<uint8_t> &data) {
uint8_t buffer[1024];
Expand All @@ -23,9 +22,7 @@ static bool read_stdin(std::vector<uint8_t> &data) {
}

// Default initialization: Override using a non-weak initialize().
__attribute__((weak)) void initialize() {
const static auto verify_handle = std::make_unique<ECCVerifyHandle>();
}
__attribute__((weak)) void initialize() {}

// This function is used by libFuzzer
extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
Expand All @@ -40,13 +37,9 @@ extern "C" int LLVMFuzzerInitialize(int *argc, char ***argv) {
return 0;
}

// Disabled under WIN32 due to clash with Cygwin's WinMain.
#ifndef WIN32
// Declare main(...) "weak" to allow for libFuzzer linking. libFuzzer provides
// the main(...) function.
__attribute__((weak))
#endif
int main(int argc, char **argv) {
__attribute__((weak)) int main(int argc, char **argv) {
initialize();
#ifdef __AFL_INIT
// Enable AFL deferred forkserver mode. Requires compilation using
Expand Down
7 changes: 7 additions & 0 deletions src/test/fuzz/script_flags.cpp
Expand Up @@ -2,15 +2,22 @@
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include <pubkey.h>
#include <script/interpreter.h>
#include <streams.h>
#include <version.h>

#include <test/fuzz/fuzz.h>

#include <memory>

/** Flags that are not forbidden by an assert */
static bool IsValidFlagCombination(uint32_t flags);

void initialize() {
static const auto verify_handle = std::make_unique<ECCVerifyHandle>();
}

void test_one_input(const std::vector<uint8_t> &buffer) {
CDataStream ds(buffer, SER_NETWORK, INIT_PROTO_VERSION);
try {
Expand Down

0 comments on commit fed8c74

Please sign in to comment.