Skip to content

tests: Add fuzzing harness for SigHasLowR(...) and ecdsa_signature_parse_der_lax(...)#19379

Merged
maflcko merged 2 commits intobitcoin:masterfrom
practicalswift:fuzzers-SigHashLowR
Aug 31, 2020
Merged

tests: Add fuzzing harness for SigHasLowR(...) and ecdsa_signature_parse_der_lax(...)#19379
maflcko merged 2 commits intobitcoin:masterfrom
practicalswift:fuzzers-SigHashLowR

Conversation

@practicalswift
Copy link
Contributor

@practicalswift practicalswift commented Jun 25, 2020

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 :)

@practicalswift practicalswift force-pushed the fuzzers-SigHashLowR branch 2 times, most recently from 6abcd62 to 07fad2c Compare June 25, 2020 14:38
@practicalswift
Copy link
Contributor Author

practicalswift commented Aug 18, 2020

@Crypt-iQ Hello fuzzing friend! Would you mind reviewing? :)

@Crypt-iQ
Copy link
Contributor

Crypt-iQ commented Aug 26, 2020

@practicalswift Will do

@practicalswift
Copy link
Contributor Author

practicalswift commented Aug 28, 2020 •
edited
Loading

@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) :)

@Crypt-iQ
Copy link
Contributor

Crypt-iQ commented Aug 31, 2020

ACK 46fcac1

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 errors
  • valgrind reports no errors on either harness

macOS:

  • ./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 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 0x104972b2a in ConsumeRandomLengthByteVector(FuzzedDataProvider&, unsigned long) util.h:38
    #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 0x1055b4ca6 in LLVMFuzzerTestOneInput fuzz.cpp:45
    #4 0x1057295a0 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) FuzzerLoop.cpp:556
    #5 0x105728ce5 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) FuzzerLoop.cpp:470
    #6 0x10572b387 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::__1::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) FuzzerLoop.cpp:765
    #7 0x10572b6e9 in fuzzer::Fuzzer::Loop(std::__1::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) FuzzerLoop.cpp:792
    #8 0x1057189dd in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) FuzzerDriver.cpp:829
    #9 0x105744e12 in main FuzzerMain.cpp:19
    #10 0x7fff73778cc8 in start+0x0 (libdyld.dylib:x86_64+0x1acc8)

maflcko
maflcko reviewed Aug 31, 2020
View reviewed changes
@maflcko maflcko merged commit 61b8c04 into bitcoin:master Aug 31, 2020
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Aug 31, 2020
Merge bitcoin#19379: tests: Add fuzzing harness for SigHasLowR(...) a…
14bf1e5 
…nd ecdsa_signature_parse_der_lax(...)

46fcac1 tests: Add fuzzing harness for ec_seckey_import_der(...) and ec_seckey_export_der(...) (practicalswift)
b667a90 tests: Add fuzzing harness for SigHasLowR(...) and ecdsa_signature_parse_der_lax(...) (practicalswift)

Pull request description:

  Add fuzzing harness for `SigHasLowR(...)` and `ecdsa_signature_parse_der_lax(...)`.

  See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/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](https://github.com/bitcoin-core/qa-assets).

  Happy fuzzing :)

ACKs for top commit:
  Crypt-iQ:
    ACK 46fcac1

Tree-SHA512: 11a4856a1efd9a04030a8c8aee2413fd5be1ea248147e649a48a55bacdf732bb48a19ee1ce2761d47d4dd61c9598aec53061b961b319ad824d539dda11a8ccf4
@practicalswift practicalswift deleted the fuzzers-SigHashLowR branch April 10, 2021 19:42
@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.

Reviewers

1 more reviewer

@maflcko maflcko maflcko left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@practicalswift @Crypt-iQ @maflcko @DrahtBot