-
Notifications
You must be signed in to change notification settings - Fork 37.7k
Closed
Labels
Description
I noticed three UBSan warnings that are reachable when loading corrupt mempool.dat
files.
$ xxd -p -r > mempool.dat-crashes-in-validation.cpp-5064 <<EOF
0100000000000000000000000004000000000000000000000000ffffffff
ffffff7f00000000000000000000000000
EOF
$ cp mempool.dat-crashes-in-validation.cpp-5064 ~/.bitcoin/regtest/mempool.dat
$ UBSAN_OPTIONS="print_stacktrace=1:halt_on_error=1:report_error_type=1" src/bitcoind -regtest
validation.cpp:5064:23: runtime error: signed integer overflow: 9223372036854775807 + 1209600 cannot be represented in type 'long'
#0 0x5573a5a07a8b in LoadMempool(CTxMemPool&, std::function<_IO_FILE* (boost::filesystem::path const&, char const*)>) src/validation.cpp:5064:23
#1 0x5573a52cce12 in ThreadImport(ChainstateManager&, std::vector<boost::filesystem::path, std::allocator<boost::filesystem::path> >) src/init.cpp:750:9
#2 0x5573a52cbef7 in AppInitMain(util::Ref const&, NodeContext&)::$_9::operator()() const src/init.cpp:1845:48
$ xxd -p -r > mempool.dat-crashes-in-util-moneystr.cpp-16a <<EOF
010000000000000000050900000000000509000000000000800000000000
0000000000000000000000800000000000
EOF
$ cp mempool.dat-crashes-in-util-moneystr.cpp-16a ~/.bitcoin/regtest/mempool.dat
$ UBSAN_OPTIONS="print_stacktrace=1:halt_on_error=1:report_error_type=1" src/bitcoind -regtest
util/moneystr.cpp:16:34: runtime error: negation of -9223372036854775808 cannot be represented in type 'CAmount' (aka 'long'); cast to an unsigned type to negate this value to itself
#0 0x5641ce3e0aec in FormatMoney[abi:cxx11](long const&) src/util/moneystr.cpp:16:34
#1 0x5641cd98a001 in CTxMemPool::PrioritiseTransaction(uint256 const&, long const&) src/txmempool.cpp:850:77
#2 0x5641cdb117b4 in LoadMempool(CTxMemPool&, std::function<_IO_FILE* (boost::filesystem::path const&, char const*)>) src/validation.cpp:5061:22
#3 0x5641cd3d6e12 in ThreadImport(ChainstateManager&, std::vector<boost::filesystem::path, std::allocator<boost::filesystem::path> >) src/init.cpp:750:9
#4 0x5641cd3d5ef7 in AppInitMain(util::Ref const&, NodeContext&)::$_9::operator()() const src/init.cpp:1845:48
$ xxd -p -r > mempool.dat-crashes-in-util-moneystr.cpp-16b <<EOF
0100000000000000000000000000000001253d8c0000000000000000006b
00000000f7000000ff00f7000000ff0000000000000000000000800000ff
0000
EOF
$ cp mempool.dat-crashes-in-util-moneystr.cpp-16b ~/.bitcoin/regtest/mempool.dat
$ UBSAN_OPTIONS="print_stacktrace=1:halt_on_error=1:report_error_type=1" src/bitcoind -regtest
util/moneystr.cpp:16:34: runtime error: negation of -9223372036854775808 cannot be represented in type 'CAmount' (aka 'long'); cast to an unsigned type to negate this value to itself
#0 0x55987d51daec in FormatMoney[abi:cxx11](long const&) src/util/moneystr.cpp:16:34
#1 0x55987cac7001 in CTxMemPool::PrioritiseTransaction(uint256 const&, long const&) src/txmempool.cpp:850:77
#2 0x55987cc4ed6f in LoadMempool(CTxMemPool&, std::function<_IO_FILE* (boost::filesystem::path const&, char const*)>) src/validation.cpp:5092:18
#3 0x55987c513e12 in ThreadImport(ChainstateManager&, std::vector<boost::filesystem::path, std::allocator<boost::filesystem::path> >) src/init.cpp:750:9
#4 0x55987c512ef7 in AppInitMain(util::Ref const&, NodeContext&)::$_9::operator()() const src/init.cpp:1845:48
The signed integer overflow can probably be avoided simply by re-arranging the terms in the conditional (nTime > nNow - nExpiryTimeout
instead of nTime + nExpiryTimeout > nNow
), and the invalid negation can likely be avoided by simply checking that the delta amount is valid before calling PrioritiseTransaction
.