Skip to content

Three UBSan warnings when loading corrupt mempool.dat files #19278

@practicalswift

Description

@practicalswift

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.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions