test: Replace file level integer overflow suppression with function level suppression #21599

pull practicalswift wants to merge 1 commits into bitcoin:master from practicalswift:remove-blanket-signed-integer-overflow-suppression-from-txmempool changing 1 files +1 −2

practicalswift commented at 9:26 PM on April 4, 2021: contributor

Replace file level (txmempool.cpp) signed integer overflow suppression with function level suppression (CTxMemPool::PrioritiseTransaction). The suppression was added yesterday in #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 #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](/bitcoin-bitcoin/0/) 0x55a09c3ce2d8 in CTxMemPool::PrioritiseTransaction(uint256 const&, long const&) /home/thomas/bitcoin/src/txmempool.cpp:847:15

test: Replace blanket UBSan signed integer overflow suppression for txmempool.cpp with specific suppression 585854ac66
JeremyRubin commented at 9:47 PM on April 4, 2021: contributor

utACK 585854a
DrahtBot added the label Tests on Apr 4, 2021
hebasto approved
hebasto commented at 11:54 PM on April 4, 2021: member

ACK 585854ac66a61fbb60849c2dfdd03f10b0150c75, I have reviewed the code and it looks OK, I agree it can be merged.
fanquake renamed this:
~~test: Replace file level (txmempool.cpp) signed integer overflow suppression with function level suppression (CTxMemPool::PrioritiseTransaction)~~
test: Replace file level integer overflow suppression with function level suppression
on Apr 5, 2021
MarcoFalke commented at 5:28 AM on April 5, 2021: member

ACK, I forgot this is possible because all other suppressions use files
MarcoFalke merged this on Apr 5, 2021
MarcoFalke closed this on Apr 5, 2021
MarcoFalke commented at 2:27 PM on April 5, 2021: member

Sorry for the edit war, but I reverted this in #21604 because due to a bug in clang we can't use symbol names and only file names.
practicalswift deleted the branch on Apr 10, 2021
DrahtBot locked this on Aug 16, 2022

Contributors

Labels

Linked (view graph)

#20626 Signed integer overflow in CTxMemPool::PrioritiseTransaction(…) reachable via RPC call prioritisetransaction #21586 test: Add missing suppression for signed-integer-overflow:txmempool.cpp