Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge #23418: Fix signed integer overflow in prioritisetransaction RPC
fa07f84 Fix signed integer overflow in prioritisetransaction RPC (MarcoFalke) fa52cf8 refactor: Replace feeDelta by m_modified_fee (MarcoFalke) Pull request description: Signed integer overflow is UB in theory, but not in practice. Still, it would be nice to avoid this UB to allow Bitcoin Core to be compiled with sanitizers such as `-ftrapv` or ubsan. It is impossible to predict when and if an overflow occurs, since the overflow caused by a prioritisetransaction RPC might only be later hit when descendant txs are added to the mempool. Since it is impossible to predict reliably, leave it up to the user to use the RPC endpoint responsibly, considering their mempool limits and usage patterns. Fixes: #20626 Fixes: #20383 Fixes: #19278 Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34146 / https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=47132 ## Steps to reproduce Build the code without the changes in this pull. Make sure to pass the sanitizer flag: ``` ./autogen.sh && ./configure --with-sanitizers=signed-integer-overflow && make clean && make -j $(nproc) ``` ### Reproduce on RPC ``` ./src/bitcoind -chain=regtest -noprinttoconsole & ./src/bitcoin-cli -chain=regtest prioritisetransaction 00000000deadbeef00000000deadbeef00000000deadbeef00000000deadbeef 0 9123456789123456789 ./src/bitcoin-cli -chain=regtest prioritisetransaction 00000000deadbeef00000000deadbeef00000000deadbeef00000000deadbeef 0 9123456789123456789 |> txmempool.cpp:920:15: runtime error: signed integer overflow: 9123456789123456789 + 9123456789123456789 cannot be represented in type 'long int' ./src/bitcoin-cli -chain=regtest stop ``` ### By fuzzing ``` wget https://github.com/bitcoin/bitcoin/files/8921302/clusterfuzz-testcase-minimized-validation_load_mempool-5599531390074880.bin.txt FUZZ=validation_load_mempool ./src/test/fuzz/fuzz ./clusterfuzz-testcase-minimized-validation_load_mempool-5599531390074880.bin.txt |> txmempool.cpp:920:15: runtime error: signed integer overflow: 7214801925397553184 + 2314885530818453536 cannot be represented in type 'long int' |> validation_load_mempool: succeeded against 1 files in 0s. ACKs for top commit: vasild: ACK fa07f84 dunxen: ACK fa07f84 LarryRuane: ACK fa07f84 Tree-SHA512: 4a357950af55a49c9113da0a50c2e743c5b752f0514dd8d16cd92bfde2f77dd0ef56aa98452626df6f7f7a5b51d1227021f6bc94091201a179f0d488ee32a0df
- Loading branch information