Signed integer overflow in CTxMemPool::PrioritiseTransaction(…) reachable via RPC call prioritisetransaction #20626

issue practicalswift opened this issue on December 11, 2020
  1. practicalswift commented at 4:06 PM on December 11, 2020: contributor

    While fuzzing the RPC interface I stumbled upon a signed integer overflow in CTxMemPool::PrioritiseTransaction(…) which is reachable via the following two prioritisetransaction RPC calls:

    $ ./autogen.sh
    $ CC=clang CXX=clang++ ./configure --with-sanitizers=address,undefined
    $ make
    $ UBSAN_OPTIONS="print_stacktrace=1" src/bitcoind &
    $ src/bitcoin-cli prioritisetransaction cafebabecafebabecafebabecafebabecafebabecafebabecafebabecafebabe 0 -9123456789123456789
    $ src/bitcoin-cli prioritisetransaction cafebabecafebabecafebabecafebabecafebabecafebabecafebabecafebabe 0 -9123456789123456789
    txmempool.cpp:832:15: runtime error: signed integer overflow: -9123456789123456789 + -9123456789123456789 cannot be represented in type 'long'
        [#0](/github-metadata-backup-bitcoin-bitcoin/0/) 0x5581f3e69c3c in CTxMemPool::PrioritiseTransaction(uint256 const&, long const&) src/txmempool.cpp:832:15
        [#1](/github-metadata-backup-bitcoin-bitcoin/1/) 0x5581f3c93852 in prioritisetransaction()::$_6::operator()(RPCHelpMan const&, JSONRPCRequest const&) const src/rpc/mining.cpp:470:36
    …
    
    SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior txmempool.cpp:832:15
    

    Nothing high priority of course, but still worth fixing :)

  2. mjdietzx commented at 4:01 PM on December 30, 2020: contributor

    Huh, I haven't been able to reproduce this. Tried with the build steps you list, but didn't see any errors:

    $ src/bitcoin-cli prioritisetransaction cafebabecafebabecafebabecafebabecafebabecafebabecafebabecafebabe 0 -9123456789123456789
    true
    $ src/bitcoin-cli prioritisetransaction cafebabecafebabecafebabecafebabecafebabecafebabecafebabecafebabe 0 -9123456789123456789
    true
    

    Also, should I be able to write a failing functional test? I've tried a few different things with no errors. ie:

    txid = wallet.send_self_transfer(from_node=self.nodes[0])['txid']
    self.nodes[0].prioritisetransaction(txid=txid, fee_delta=-9123456789123456789)
    self.nodes[0].prioritisetransaction(txid=txid, fee_delta=-9123456789123456789)
    

    or:

    txid = wallet.send_self_transfer(from_node=self.nodes[0])['txid']
    self.nodes[0].prioritisetransaction(txid='cafebabecafebabecafebabecafebabecafebabecafebabecafebabecafebabe', fee_delta=-9123456789123456789)
    self.nodes[0].prioritisetransaction(txid='cafebabecafebabecafebabecafebabecafebabecafebabecafebabecafebabe', fee_delta=-9123456789123456789)
    

    prioritisetransaction returns True with no errors in both of those cases

  3. practicalswift commented at 9:50 PM on January 12, 2021: contributor

    @mjdietzx Oh, that's weird! I just re-tried against current master using the commands below and was able to reproduce. Could you re-try using the commands below? What version of Clang are you using?

    $ git clone https://github.com/bitcoin/bitcoin
    $ cd bitcoin/
    $ ./autogen.sh
    $ CC=clang CXX=clang++ ./configure --with-sanitizers=address,undefined --disable-wallet
    $ make
    $ UBSAN_OPTIONS="print_stacktrace=1" src/bitcoind -regtest &
    $ src/bitcoin-cli -regtest prioritisetransaction cafebabecafebabecafebabecafebabecafebabecafebabecafebabecafebabe 0 -9123456789123456789
    $ src/bitcoin-cli -regtest prioritisetransaction cafebabecafebabecafebabecafebabecafebabecafebabecafebabecafebabe 0 -9123456789123456789
    txmempool.cpp:832:15: runtime error: signed integer overflow: -8723795798198180713 + -9123456789123456789 cannot be represented in type 'long'
    
  4. jonatack commented at 12:03 AM on January 13, 2021: contributor

    Reproduced on current master @ 7b975639ef93b50537a3ec6326b54d7218afc8da

    $ clang --version
    clang version 9.0.1-15+b1 
    Target: x86_64-pc-linux-gnu
    Thread model: posix
    InstalledDir: /usr/bin
    

    <details><summary>debug log output</summary><p>

    2021-01-12T23:52:08Z PrioritiseTransaction: cafebabecafebabecafebabecafebabecafebabecafebabecafebabecafebabe feerate += -91234567891.23456789
    txmempool.cpp:832:15: runtime error: signed integer overflow: -9123456789123456789 + -9123456789123456789 cannot be represented in type 'long'
        [#0](/github-metadata-backup-bitcoin-bitcoin/0/) 0x5624c3cb287c in CTxMemPool::PrioritiseTransaction(uint256 const&, long const&) /home/jon/projects/bitcoin/bitcoin/src/txmempool.cpp:832:15
        [#1](/github-metadata-backup-bitcoin-bitcoin/1/) 0x5624c3a69eb8 in prioritisetransaction()::$_6::operator()(RPCHelpMan const&, JSONRPCRequest const&) const /home/jon/projects/bitcoin/bitcoin/src/rpc/mining.cpp:470:36
        [#2](/github-metadata-backup-bitcoin-bitcoin/2/) 0x5624c3a69eb8 in UniValue std::__invoke_impl<UniValue, prioritisetransaction()::$_6&, RPCHelpMan const&, JSONRPCRequest const&>(std::__invoke_other, prioritisetransaction()::$_6&, RPCHelpMan const&, JSONRPCRequest const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/invoke.h:60:14
        [#3](/github-metadata-backup-bitcoin-bitcoin/3/) 0x5624c3a69a88 in std::enable_if<is_invocable_r_v<UniValue, prioritisetransaction()::$_6&, RPCHelpMan const&, JSONRPCRequest const&>, UniValue>::type std::__invoke_r<UniValue, prioritisetransaction()::$_6&, RPCHelpMan const&, JSONRPCRequest const&>(prioritisetransaction()::$_6&, RPCHelpMan const&, JSONRPCRequest const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/invoke.h:113:9
        [#4](/github-metadata-backup-bitcoin-bitcoin/4/) 0x5624c3a696af in std::_Function_handler<UniValue (RPCHelpMan const&, JSONRPCRequest const&), prioritisetransaction()::$_6>::_M_invoke(std::_Any_data const&, RPCHelpMan const&, JSONRPCRequest const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/std_function.h:291:9
        [#5](/github-metadata-backup-bitcoin-bitcoin/5/) 0x5624c3966ff8 in std::function<UniValue (RPCHelpMan const&, JSONRPCRequest const&)>::operator()(RPCHelpMan const&, JSONRPCRequest const&) const /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/std_function.h:622:14
        [#6](/github-metadata-backup-bitcoin-bitcoin/6/) 0x5624c3965f88 in RPCHelpMan::HandleRequest(JSONRPCRequest const&) /home/jon/projects/bitcoin/bitcoin/src/./rpc/util.h:342:16
        [#7](/github-metadata-backup-bitcoin-bitcoin/7/) 0x5624c3a09034 in CRPCCommand::CRPCCommand(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, RPCHelpMan (*)(), std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >)::'lambda'(JSONRPCRequest const&, UniValue&, bool)::operator()(JSONRPCRequest const&, UniValue&, bool) const /home/jon/projects/bitcoin/bitcoin/src/./rpc/server.h:110:91
        [#8](/github-metadata-backup-bitcoin-bitcoin/8/) 0x5624c3a08d97 in bool std::__invoke_impl<bool, CRPCCommand::CRPCCommand(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, RPCHelpMan (*)(), std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >)::'lambda'(JSONRPCRequest const&, UniValue&, bool)&, JSONRPCRequest const&, UniValue&, bool>(std::__invoke_other, CRPCCommand::CRPCCommand(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, RPCHelpMan (*)(), std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >)::'lambda'(JSONRPCRequest const&, UniValue&, bool)&, JSONRPCRequest const&, UniValue&, bool&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/invoke.h:60:14
        [#9](/github-metadata-backup-bitcoin-bitcoin/9/) 0x5624c3a08b82 in std::enable_if<is_invocable_r_v<bool, CRPCCommand::CRPCCommand(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, RPCHelpMan (*)(), std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >)::'lambda'(JSONRPCRequest const&, UniValue&, bool)&, JSONRPCRequest const&, UniValue&, bool>, bool>::type std::__invoke_r<bool, CRPCCommand::CRPCCommand(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, RPCHelpMan (*)(), std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >)::'lambda'(JSONRPCRequest const&, UniValue&, bool)&, JSONRPCRequest const&, UniValue&, bool>(CRPCCommand::CRPCCommand(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, RPCHelpMan (*)(), std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >)::'lambda'(JSONRPCRequest const&, UniValue&, bool)&, JSONRPCRequest const&, UniValue&, bool&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/invoke.h:113:9
        [#10](/github-metadata-backup-bitcoin-bitcoin/10/) 0x5624c3a086f7 in std::_Function_handler<bool (JSONRPCRequest const&, UniValue&, bool), CRPCCommand::CRPCCommand(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, RPCHelpMan (*)(), std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >)::'lambda'(JSONRPCRequest const&, UniValue&, bool)>::_M_invoke(std::_Any_data const&, JSONRPCRequest const&, UniValue&, bool&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/std_function.h:291:9
        [#11](/github-metadata-backup-bitcoin-bitcoin/11/) 0x5624c37fe486 in std::function<bool (JSONRPCRequest const&, UniValue&, bool)>::operator()(JSONRPCRequest const&, UniValue&, bool) const /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/std_function.h:622:14
        [#12](/github-metadata-backup-bitcoin-bitcoin/12/) 0x5624c3ba61ec in ExecuteCommand(CRPCCommand const&, JSONRPCRequest const&, UniValue&, bool) /home/jon/projects/bitcoin/bitcoin/src/rpc/server.cpp:466:20
        [#13](/github-metadata-backup-bitcoin-bitcoin/13/) 0x5624c3ba5ac0 in CRPCTable::execute(JSONRPCRequest const&) const /home/jon/projects/bitcoin/bitcoin/src/rpc/server.cpp:449:17
        [#14](/github-metadata-backup-bitcoin-bitcoin/14/) 0x5624c3ef9471 in HTTPReq_JSONRPC(util::Ref const&, HTTPRequest*) /home/jon/projects/bitcoin/bitcoin/src/httprpc.cpp:201:40
        [#15](/github-metadata-backup-bitcoin-bitcoin/15/) 0x5624c3ef8b9e in StartHTTPRPC(util::Ref const&)::$_0::operator()(HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const /home/jon/projects/bitcoin/bitcoin/src/httprpc.cpp:297:81
        [#16](/github-metadata-backup-bitcoin-bitcoin/16/) 0x5624c3ef8b9e in bool std::__invoke_impl<bool, StartHTTPRPC(util::Ref const&)::$_0&, HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&>(std::__invoke_other, StartHTTPRPC(util::Ref const&)::$_0&, HTTPRequest*&&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/invoke.h:60:14
        [#17](/github-metadata-backup-bitcoin-bitcoin/17/) 0x5624c3ef897e in std::enable_if<is_invocable_r_v<bool, StartHTTPRPC(util::Ref const&)::$_0&, HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&>, bool>::type std::__invoke_r<bool, StartHTTPRPC(util::Ref const&)::$_0&, HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&>(StartHTTPRPC(util::Ref const&)::$_0&, HTTPRequest*&&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/invoke.h:113:9
        [#18](/github-metadata-backup-bitcoin-bitcoin/18/) 0x5624c3ef84ee in std::_Function_handler<bool (HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&), StartHTTPRPC(util::Ref const&)::$_0>::_M_invoke(std::_Any_data const&, HTTPRequest*&&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/std_function.h:291:9
        [#19](/github-metadata-backup-bitcoin-bitcoin/19/) 0x5624c3f1d640 in std::function<bool (HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)>::operator()(HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/std_function.h:622:14
        [#20](/github-metadata-backup-bitcoin-bitcoin/20/) 0x5624c3f1d258 in HTTPWorkItem::operator()() /home/jon/projects/bitcoin/bitcoin/src/httpserver.cpp:49:9
        [#21](/github-metadata-backup-bitcoin-bitcoin/21/) 0x5624c3f27cbc in WorkQueue<HTTPClosure>::Run() /home/jon/projects/bitcoin/bitcoin/src/httpserver.cpp:108:13
        [#22](/github-metadata-backup-bitcoin-bitcoin/22/) 0x5624c3f0deb3 in HTTPWorkQueueRun(WorkQueue<HTTPClosure>*, int) /home/jon/projects/bitcoin/bitcoin/src/httpserver.cpp:336:12
        [#23](/github-metadata-backup-bitcoin-bitcoin/23/) 0x5624c3f2e880 in void std::__invoke_impl<void, void (*)(WorkQueue<HTTPClosure>*, int), WorkQueue<HTTPClosure>*, int>(std::__invoke_other, void (*&&)(WorkQueue<HTTPClosure>*, int), WorkQueue<HTTPClosure>*&&, int&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/invoke.h:60:14
        [#24](/github-metadata-backup-bitcoin-bitcoin/24/) 0x5624c3f2e70e in std::__invoke_result<void (*)(WorkQueue<HTTPClosure>*, int), WorkQueue<HTTPClosure>*, int>::type std::__invoke<void (*)(WorkQueue<HTTPClosure>*, int), WorkQueue<HTTPClosure>*, int>(void (*&&)(WorkQueue<HTTPClosure>*, int), WorkQueue<HTTPClosure>*&&, int&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/invoke.h:95:14
        [#25](/github-metadata-backup-bitcoin-bitcoin/25/) 0x5624c3f2e4d5 in void std::thread::_Invoker<std::tuple<void (*)(WorkQueue<HTTPClosure>*, int), WorkQueue<HTTPClosure>*, int> >::_M_invoke<0ul, 1ul, 2ul>(std::_Index_tuple<0ul, 1ul, 2ul>) /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/thread:264:13
        [#26](/github-metadata-backup-bitcoin-bitcoin/26/) 0x5624c3f2deb5 in std::thread::_Invoker<std::tuple<void (*)(WorkQueue<HTTPClosure>*, int), WorkQueue<HTTPClosure>*, int> >::operator()() /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/thread:271:11
        [#27](/github-metadata-backup-bitcoin-bitcoin/27/) 0x5624c3f2deb5 in std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (*)(WorkQueue<HTTPClosure>*, int), WorkQueue<HTTPClosure>*, int> > >::_M_run() /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/thread:215:13
        [#28](/github-metadata-backup-bitcoin-bitcoin/28/) 0x7fc51f0c9ecf  (/lib/x86_64-linux-gnu/libstdc++.so.6+0xceecf)
        [#29](/github-metadata-backup-bitcoin-bitcoin/29/) 0x7fc51f34fea6 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x8ea6)
        [#30](/github-metadata-backup-bitcoin-bitcoin/30/) 0x7fc51edc2dee in clone (/lib/x86_64-linux-gnu/libc.so.6+0xfddee)
    
    SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior txmempool.cpp:832:15 in 
    2021-01-12T23:52:19Z PrioritiseTransaction: cafebabecafebabecafebabecafebabecafebabecafebabecafebabecafebabe feerate += -91234567891.23456789
    

    </p></details>

  5. MarcoFalke commented at 9:57 AM on January 14, 2021: member

    Fixed in #20383

  6. practicalswift cross-referenced this on Apr 4, 2021 from issue test: Replace file level integer overflow suppression with function level suppression by practicalswift
  7. MarcoFalke referenced this in commit 824eea5643 on Apr 5, 2021
  8. MarcoFalke commented at 1:49 PM on April 6, 2021: member

    Looks like fuzzing with -ftrapv will currently fail due to this

  9. MarcoFalke commented at 10:33 AM on May 10, 2021: member
  10. MarcoFalke cross-referenced this on May 10, 2021 from issue Three UBSan warnings when loading corrupt mempool.dat files by practicalswift
  11. practicalswift closed this on Oct 28, 2021

  12. MarcoFalke cross-referenced this on Nov 2, 2021 from issue Fix signed integer overflow in prioritisetransaction RPC by MarcoFalke
  13. MarcoFalke referenced this in commit dde7205c57 on Jun 27, 2022
  14. sidhujag referenced this in commit 758481e22d on Jun 27, 2022
  15. bitcoin locked this on Oct 30, 2022

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-20 06:54 UTC