Add fuzzing harness for SigHasLowR(...) and ecdsa_signature_parse_der_lax(...).
See doc/fuzzing.md for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the Bitcoin Core fuzzing corpus repo.
Happy fuzzing :)
@Crypt-iQ Hello fuzzing friend! Would you mind reviewing? :)
@practicalswift Will do
@MarcoFalke Would you mind reviewing? FWIW this PR touches only src/test/fuzz/ (edit: not entirely correct -- it also makes the relevant functions visible outside of their translation unit to allow for testing) :)
ACK 46fcac1e4b9e0b1026bc0b663582148b2fd60390
Coverage from just these harnesses: https://crypt-iq.github.io/19379_fuzz_cov/
Ubuntu:
./configure --enable-fuzz --with-sanitizers=address,undefined,integer,fuzzer reports no errorsmacOS:
./configure --enable-fuzz --with-sanitizers=address,fuzzer --disable-asm reports no errors./configure --enable-fuzz --with-sanitizers=undefined,integer,fuzzer --disable-asm complains for the secp256k1_ecdsa_signature_parse_der_lax harness:/usr/local/opt/llvm/bin/../include/c++/v1/memory:1876:35: runtime error: implicit conversion from type 'char' of value -35 (8-bit, signed) to type 'unsigned char' changed the value to 221 (8-bit, unsigned)
[#0](/bitcoin-bitcoin/0/) 0x1049734d1 in std::__1::enable_if<__is_cpp17_forward_iterator<std::__1::__wrap_iter<char const*> >::value, void>::type std::__1::vector<unsigned char, std::__1::allocator<unsigned char> >::__construct_at_end<std::__1::__wrap_iter<char const*> >(std::__1::__wrap_iter<char const*>, std::__1::__wrap_iter<char const*>, unsigned long) vector:1076
[#1](/bitcoin-bitcoin/1/) 0x104972b2a in ConsumeRandomLengthByteVector(FuzzedDataProvider&, unsigned long) util.h:38
[#2](/bitcoin-bitcoin/2/) 0x104972482 in test_one_input(std::__1::vector<unsigned char, std::__1::allocator<unsigned char> > const&) secp256k1_ecdsa_signature_parse_der_lax.cpp:20
[#3](/bitcoin-bitcoin/3/) 0x1055b4ca6 in LLVMFuzzerTestOneInput fuzz.cpp:45
[#4](/bitcoin-bitcoin/4/) 0x1057295a0 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) FuzzerLoop.cpp:556
[#5](/bitcoin-bitcoin/5/) 0x105728ce5 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) FuzzerLoop.cpp:470
[#6](/bitcoin-bitcoin/6/) 0x10572b387 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::__1::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) FuzzerLoop.cpp:765
[#7](/bitcoin-bitcoin/7/) 0x10572b6e9 in fuzzer::Fuzzer::Loop(std::__1::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) FuzzerLoop.cpp:792
[#8](/bitcoin-bitcoin/8/) 0x1057189dd in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) FuzzerDriver.cpp:829
[#9](/bitcoin-bitcoin/9/) 0x105744e12 in main FuzzerMain.cpp:19
[#10](/bitcoin-bitcoin/10/) 0x7fff73778cc8 in start+0x0 (libdyld.dylib:x86_64+0x1acc8)
23 | @@ -24,7 +24,7 @@ secp256k1_context* secp256k1_context_verify = nullptr; 24 | * strict DER before being passed to this module, and we know it supports all 25 | * violations present in the blockchain before that point. 26 | */ 27 | -static int ecdsa_signature_parse_der_lax(const secp256k1_context* ctx, secp256k1_ecdsa_signature* sig, const unsigned char *input, size_t inputlen) { 28 | +int ecdsa_signature_parse_der_lax(const secp256k1_context* ctx, secp256k1_ecdsa_signature* sig, const unsigned char *input, size_t inputlen) {
why can't you call CPubKey::CheckLowS, which directly calls this?
I want to fuzz both in order to a.) maximize coverage, and b.) to avoid assuming anything about the interdependence of these two function (which may change over time).
30 | @@ -31,7 +31,7 @@ static secp256k1_context* secp256k1_context_sign = nullptr; 31 | * 32 | * out32 must point to an output buffer of length at least 32 bytes. 33 | */ 34 | -static int ec_seckey_import_der(const secp256k1_context* ctx, unsigned char *out32, const unsigned char *seckey, size_t seckeylen) { 35 | +int ec_seckey_import_der(const secp256k1_context* ctx, unsigned char *out32, const unsigned char *seckey, size_t seckeylen) {
why not CKey::Load?
Same here: I want to fuzz both in order to a.) maximize coverage, and b.) to avoid assuming anything about the interdependence of these two function (which may change over time).
87 | @@ -88,7 +88,7 @@ static int ec_seckey_import_der(const secp256k1_context* ctx, unsigned char *out 88 | * will be set to the number of bytes used in the buffer. 89 | * key32 must point to a 32-byte raw private key. 90 | */ 91 | -static int ec_seckey_export_der(const secp256k1_context *ctx, unsigned char *seckey, size_t *seckeylen, const unsigned char *key32, bool compressed) { 92 | +int ec_seckey_export_der(const secp256k1_context *ctx, unsigned char *seckey, size_t *seckeylen, const unsigned char *key32, bool compressed) {
why not CKey::GetPrivKey?
I want to fuzz both in order to a.) maximize coverage, and b.) to avoid assuming anything about the interdependence of these two function (which may change over time).