Skip to content

fuzz: Add missing CheckTransaction before CheckTxInputs #21970

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
maflcko merged 2 commits into from
May 19, 2021
Merged

fuzz: Add missing CheckTransaction before CheckTxInputs #21970

maflcko merged 2 commits into from
May 19, 2021

Conversation

@maflcko
Copy link
Member

@maflcko maflcko commented May 17, 2021 •
edited
Loading

@DrahtBot DrahtBot added the Tests label May 17, 2021
glozow
glozow reviewed May 17, 2021
View reviewed changes
Copy link
Member

@glozow glozow left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Concept ACK
Please excuse my nosiness, I can't see the error but makes sense to me. We'd want to call CheckTransaction() to make sure it's well-formed first 🤷

@maflcko
Copy link
Member Author

maflcko commented May 17, 2021

I can't see the error

It should be possible to reproduce with the reproducer input in the OP

@adamjonas
Copy link
Member

adamjonas commented May 17, 2021

Tested fae4ee5.

Was able to reproduce with master (c857148) 
python infra/helper.py reproduce bitcoin-core coins_view clusterfuzz-testcase-minimized-coins_view-6109460079706112
Running: docker run --rm --privileged -i -v /home/jonas/oss-fuzz/build/out/bitcoin-core:/out -v /home/jonas/clusterfuzz-testcase-minimized-coins_view-6109460079706112:/testcase -t gcr.io/oss-fuzz-base/bas
e-runner reproduce coins_view -runs=100
+ FUZZER=coins_view
+ shift
+ '[' '!' -v TESTCASE ']'
+ TESTCASE=/testcase
+ '[' '!' -f /testcase ']'
+ export RUN_FUZZER_MODE=interactive
+ RUN_FUZZER_MODE=interactive
+ export FUZZING_ENGINE=libfuzzer
+ FUZZING_ENGINE=libfuzzer
+ export SKIP_SEED_CORPUS=1
+ SKIP_SEED_CORPUS=1
+ run_fuzzer coins_view -runs=100 /testcase
/out/coins_view -rss_limit_mb=2560 -timeout=25 -runs=100 /testcase < /dev/null
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 310929130
INFO: Loaded 1 modules   (177026 inline 8-bit counters): 177026 [0x55862ca7d158, 0x55862caa84da),
INFO: Loaded 1 PC tables (177026 PCs): 177026 [0x55862caa84e0,0x55862cd5bd00),
/out/coins_view: Running 1 inputs 100 time(s) each.
Running: /testcase
libc++abi: terminating with uncaught exception of type std::runtime_error: GetValueOut: value out of range
AddressSanitizer:DEADLYSIGNAL
=================================================================
=================================================================
==13==ERROR: AddressSanitizer: ABRT on unknown address 0x00000000000d (pc 0x7f321e8e7438 bp 0x7ffc67fe9cf0 sp 0x7ffc67fe9588 T0)
SCARINESS: 10 (signal)
    #0 0x7f321e8e7438 in raise (/lib/x86_64-linux-gnu/libc.so.6+0x35438)
    #1 0x7f321e8e9039 in abort (/lib/x86_64-linux-gnu/libc.so.6+0x37039)
    #2 0x55862bf98fb5 in abort_message (/out/coins_view+0x1ad0fb5)
    #3 0x55862bfa4e9e in demangling_terminate_handler() (/out/coins_view+0x1adce9e)
    #4 0x55862bf98a22 in std::__terminate(void (*)()) (/out/coins_view+0x1ad0a22)
    #5 0x55862bf9a775 in __cxxabiv1::failed_throw(__cxxabiv1::__cxa_exception*) (/out/coins_view+0x1ad2775)
    #6 0x55862bf9a70f in __cxa_throw (/out/coins_view+0x1ad270f)
    #7 0x55862bd7d512 in CTransaction::GetValueOut() const /src/bitcoin-core/src/primitives/transaction.cpp:88:13
    #8 0x55862b0f30ab in Consensus::CheckTxInputs(CTransaction const&, TxValidationState&, CCoinsViewCache const&, int, long&) /src/bitcoin-core/src/consensus/tx_verify.cpp:186:34
    #9 0x55862adfaf4c in coins_view_fuzz_target(Span<unsigned char const>)::$_12::operator()() const /src/bitcoin-core/src/test/fuzz/coins_view.cpp:233:23
    #10 0x55862ade8b6f in void CallOneOf<coins_view_fuzz_target(Span<unsigned char const>)::$_10, coins_view_fuzz_target(Span<unsigned char const>)::$_11, coins_view_fuzz_target(Span<unsigned char const>)::$_12, coins_view_fuzz_target(Span<unsigned char const>)::$_13, coins_view_fuzz_target(Span<unsigned char const>)::$_14, coins_view_fuzz_target(Span<unsigned char const>)::$_15, coins_view_fuzz_target(Span<unsigned char const>)::$_16>(FuzzedDataProvider&, coins_view_fuzz_target(Span<unsigned char const>)::$_10, coins_view_fuzz_target(Span<unsigned char const>)::$_11, coins_view_fuzz_target(Span<unsigned char const>)::$_12, coins_view_fuzz_target(Span<unsigned char const>)::$_13, coins_view_fuzz_target(Span<unsigned char const>)::$_14, coins_view_fuzz_target(Span<unsigned char const>)::$_15, coins_view_fuzz_target(Span<unsigned char const>)::$_16) /src/bitcoin-core/src/./test/fuzz/util.h:47:34
    #11 0x55862ade791a in coins_view_fuzz_target(Span<unsigned char const>) /src/bitcoin-core/src/test/fuzz/coins_view.cpp:191:9
    #12 0x55862ad0a8d6 in decltype(std::__1::forward<void (*&)(Span<unsigned char const>)>(fp)(std::__1::forward<Span<unsigned char const> >(fp0))) std::__1::__invoke<void (*&)(Span<unsigned char const>), Span<unsigned char const> >(void (*&)(Span<unsigned char const>), Span<unsigned char const>&&) /usr/local/bin/../include/c++/v1/type_traits:3679:1
    #13 0x55862ad0a7d1 in void std::__1::__invoke_void_return_wrapper<void>::__call<void (*&)(Span<unsigned char const>), Span<unsigned char const> >(void (*&)(Span<unsigned char const>), Span<unsigned char const>&&) /usr/local/bin/../include/c++/v1/__functional_base:348:9
    #14 0x55862ad0a721 in std::__1::__function::__alloc_func<void (*)(Span<unsigned char const>), std::__1::allocator<void (*)(Span<unsigned char const>)>, void (Span<unsigned char const>)>::operator()(Span<unsigned char const>&&) /usr/local/bin/../include/c++/v1/functional:1558:16
    #15 0x55862ad07a0b in std::__1::__function::__func<void (*)(Span<unsigned char const>), std::__1::allocator<void (*)(Span<unsigned char const>)>, void (Span<unsigned char const>)>::operator()(Span<unsigned char const>&&) /usr/local/bin/../include/c++/v1/functional:1732:12
    #16 0x55862bd6ec95 in std::__1::__function::__value_func<void (Span<unsigned char const>)>::operator()(Span<unsigned char const>&&) const /usr/local/bin/../include/c++/v1/functional:1885:16
    #17 0x55862bd69b35 in std::__1::function<void (Span<unsigned char const>)>::operator()(Span<unsigned char const>) const /usr/local/bin/../include/c++/v1/functional:2560:12
    #18 0x55862bd698a1 in LLVMFuzzerTestOneInput /src/bitcoin-core/src/test/fuzz/fuzz.cpp:74:5
    #19 0x55862ac07d53 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:599:15
    #20 0x55862abf1b62 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:323:6
    #21 0x55862abf79aa in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:856:9
    #22 0x55862ac23692 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
    #23 0x7f321e8d283f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2083f)
    #24 0x55862abcc818 in _start (/out/coins_view+0x704818)

DEDUP_TOKEN: raise--abort--abort_message
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: ABRT (/lib/x86_64-linux-gnu/libc.so.6+0x35438) in raise
==13==ABORTING
Verified there is no crash with fae4ee5 
python infra/helper.py reproduce bitcoin-core coins_view clusterfuzz-testcase-minimized-coins_view-6109460079706112
Running: docker run --rm --privileged -i -v /home/jonas/oss-fuzz/build/out/bitcoin-core:/out -v /home/jonas/clusterfuzz-testcase-minimized-coins_view-6109460079706112:/testcase -t gcr.io/oss-fuzz-base/base-runner reproduce coins_view -runs=100
+ FUZZER=coins_view
+ shift
+ '[' '!' -v TESTCASE ']'
+ TESTCASE=/testcase
+ '[' '!' -f /testcase ']'
+ export RUN_FUZZER_MODE=interactive
+ RUN_FUZZER_MODE=interactive
+ export FUZZING_ENGINE=libfuzzer
+ FUZZING_ENGINE=libfuzzer
+ export SKIP_SEED_CORPUS=1
+ SKIP_SEED_CORPUS=1
+ run_fuzzer coins_view -runs=100 /testcase
/out/coins_view -rss_limit_mb=2560 -timeout=25 -runs=100 /testcase < /dev/null
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 2597972927
INFO: Loaded 1 modules   (177031 inline 8-bit counters): 177031 [0x5619464791d8, 0x5619464a455f),
INFO: Loaded 1 PC tables (177031 PCs): 177031 [0x5619464a4560,0x561946757dd0),
/out/coins_view: Running 1 inputs 100 time(s) each.
Running: /testcase
Executed /testcase in 60 ms
***
*** NOTE: fuzzing was not performed, you have only
***       executed the target code on a fixed set of inputs.

@maflcko
Copy link
Member Author

maflcko commented May 17, 2021

Of course you can reproduce this inside Docker, if you want, but the recommended way to reproduce is to use "your build and test system" (https://google.github.io/oss-fuzz/advanced-topics/reproducing/#fuzz-target-bugs). This would be https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md .

Docker can always be used as a backup in case it it not trivial to build with the sanitizers for the given architecture.

@practicalswift
Copy link
Contributor

practicalswift commented May 18, 2021

cr ACK fae4ee5: patch looks correct :)

@maflcko maflcko merged commit 7d19c85 into bitcoin:master May 19, 2021
@maflcko maflcko deleted the 2105-fuzzTxFixes branch May 19, 2021 19:27
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request May 19, 2021
Merge bitcoin#21970: fuzz: Add missing CheckTransaction before CheckT…
ec73d49 
…xInputs
gwillen pushed a commit to ElementsProject/elements that referenced this pull request Jun 1, 2022
@apoelstra
Merge 7d19c85 into merged_master (Bitcoin PR bitcoin/bitcoin#21970)
ff432ef
@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

@glozow glozow glozow left review comments

Assignees

No one assigned

Labels

Tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@maflcko @adamjonas @practicalswift @glozow @DrahtBot