Skip to content
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.

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

test: Add missing suppression for signed-integer-overflow:txmempool.cpp #21586

Merged
merged 1 commit into from
Apr 4, 2021

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Apr 3, 2021

Otherwise the fuzzer will crash:

txmempool.cpp:847:15: runtime error: signed integer overflow: 8138645194045128704 + 4611686018427387904 cannot be represented in type 'long'
    #0 0x558ff1838d4c in CTxMemPool::PrioritiseTransaction(uint256 const&, long const&) /root/fuzz_dir/scratch/fuzz_gen/code/src/txmempool.cpp:847:15
    #1 0x558ff196e723 in LoadMempool(CTxMemPool&, CChainState&, std::function<_IO_FILE* (boost::filesystem::path const&, char const*)>) /root/fuzz_dir/scratch/fuzz_gen/code/src/validation.cpp:5053:22
    #2 0x558ff13f37ab in validation_load_mempool_fuzz_target(Span<unsigned char const>) /root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/validation_load_mempool.cpp:32:11
    #3 0x558ff1083378 in std::_Function_handler<void (Span<unsigned char const>), void (*)(Span<unsigned char const>)>::_M_invoke(std::_Any_data const&, Span<unsigned char const>&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:300:2
    #4 0x558ff22a749d in std::function<void (Span<unsigned char const>)>::operator()(Span<unsigned char const>) const /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:688:14
    #5 0x558ff22a70e8 in LLVMFuzzerTestOneInput /root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz.cpp:63:5
    #6 0x558ff0f83543 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) fuzzer.o
    #7 0x558ff0f6d442 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) fuzzer.o
    #8 0x558ff0f7323a in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) fuzzer.o
    #9 0x558ff0f9ef82 in main (/root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz+0x196df82)
    #10 0x7f1237f310b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16
    #11 0x558ff0f4816d in _start (/root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz+0x191716d)

@maflcko
Copy link
Member Author

maflcko commented Apr 4, 2021

Going to merge this to unbreak the fuzzers. The only alternative I see to this is removing the fuzz test or marking it hidden.

@maflcko maflcko merged commit 590e49c into bitcoin:master Apr 4, 2021
@maflcko maflcko deleted the 2104-testSupp branch April 4, 2021 05:46
maflcko pushed a commit to maflcko/bitcoin-core that referenced this pull request Apr 5, 2021
…ssion with function level suppression

585854a test: Replace blanket UBSan signed integer overflow suppression for txmempool.cpp with specific suppression (practicalswift)

Pull request description:

  Replace file level (`txmempool.cpp`) signed integer overflow suppression with function level suppression (`CTxMemPool::PrioritiseTransaction`). The suppression was added yesterday in bitcoin#21586.

  Rationale: To avoid risk hiding other signed integer overflows in `txmempool.cpp`.

  Obviously it would be better if this signed integer overflow fixed instead of suppressed - see details bitcoin#20626. Any taker? :)

  To hit the issue via fuzzing:

  ```
  $ UBSAN_OPTIONS="print_stacktrace=1:halt_on_error=1:report_error_type=1" FUZZ=validation_load_mempool src/test/fuzz/fuzz
  INFO: Seed: 1184244493
  INFO: Loaded 1 modules   (634418 inline 8-bit counters): 634418 [0x55a09fdfbf98, 0x55a09fe96dca),
  INFO: Loaded 1 PC tables (634418 PCs): 634418 [0x55a09fe96dd0,0x55a0a08450f0),
  INFO:     1264 files found in mempool/
  INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 1040698 bytes
  INFO: seed corpus: files: 1264 min: 1b max: 1040698b total: 15997133b rss: 197Mb
  txmempool.cpp:847:15: runtime error: signed integer overflow: -7211388903327006720 + -7211353718954917888 cannot be represented in type 'long'
      #0 0x55a09c3ce2d8 in CTxMemPool::PrioritiseTransaction(uint256 const&, long const&) /home/thomas/bitcoin/src/txmempool.cpp:847:15
  ```

ACKs for top commit:
  JeremyRubin:
    utACK 585854a
  hebasto:
    ACK 585854a, I have reviewed the code and it looks OK, I agree it can be merged.

Tree-SHA512: 5a343f028c1e1a1aba3b51a0eced605849184891ffafecb3cd2b424c6cfea01afd7c2672274936b0bac646075ec066408a570bf6b34bc9b87399a53ce20d8a23
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Apr 5, 2021
…-overflow:txmempool.cpp

fad8a97 test: Add missing suppression for signed-integer-overflow:txmempool.cpp (MarcoFalke)

Pull request description:

  Otherwise the fuzzer will crash:

  ```
  txmempool.cpp:847:15: runtime error: signed integer overflow: 8138645194045128704 + 4611686018427387904 cannot be represented in type 'long'
      #0 0x558ff1838d4c in CTxMemPool::PrioritiseTransaction(uint256 const&, long const&) /root/fuzz_dir/scratch/fuzz_gen/code/src/txmempool.cpp:847:15
      #1 0x558ff196e723 in LoadMempool(CTxMemPool&, CChainState&, std::function<_IO_FILE* (boost::filesystem::path const&, char const*)>) /root/fuzz_dir/scratch/fuzz_gen/code/src/validation.cpp:5053:22
      #2 0x558ff13f37ab in validation_load_mempool_fuzz_target(Span<unsigned char const>) /root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/validation_load_mempool.cpp:32:11
      #3 0x558ff1083378 in std::_Function_handler<void (Span<unsigned char const>), void (*)(Span<unsigned char const>)>::_M_invoke(std::_Any_data const&, Span<unsigned char const>&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:300:2
      #4 0x558ff22a749d in std::function<void (Span<unsigned char const>)>::operator()(Span<unsigned char const>) const /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:688:14
      #5 0x558ff22a70e8 in LLVMFuzzerTestOneInput /root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz.cpp:63:5
      #6 0x558ff0f83543 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) fuzzer.o
      #7 0x558ff0f6d442 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) fuzzer.o
      #8 0x558ff0f7323a in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) fuzzer.o
      #9 0x558ff0f9ef82 in main (/root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz+0x196df82)
      #10 0x7f1237f310b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16
      #11 0x558ff0f4816d in _start (/root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz+0x191716d)

Top commit has no ACKs.

Tree-SHA512: 94c13771054b4acfb83e3dcfa09beb3f9d0ca0e025d3993cdf2e46df6456f227565b31fd4377b8dd86c567aeee800f293ac57a470c6f5f81e9177d460e7bd705
@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.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants