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

issue practicalswift openend 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:

     0$ ./autogen.sh
     1$ CC=clang CXX=clang++ ./configure --with-sanitizers=address,undefined
     2$ make
     3$ UBSAN_OPTIONS="print_stacktrace=1" src/bitcoind &
     4$ src/bitcoin-cli prioritisetransaction cafebabecafebabecafebabecafebabecafebabecafebabecafebabecafebabe 0 -9123456789123456789
     5$ src/bitcoin-cli prioritisetransaction cafebabecafebabecafebabecafebabecafebabecafebabecafebabecafebabe 0 -9123456789123456789
     6txmempool.cpp:832:15: runtime error: signed integer overflow: -9123456789123456789 + -9123456789123456789 cannot be represented in type 'long'
     7    [#0](/bitcoin-bitcoin/0/) 0x5581f3e69c3c in CTxMemPool::PrioritiseTransaction(uint256 const&, long const&) src/txmempool.cpp:832:15
     8    [#1](/bitcoin-bitcoin/1/) 0x5581f3c93852 in prioritisetransaction()::$_6::operator()(RPCHelpMan const&, JSONRPCRequest const&) const src/rpc/mining.cpp:470:36
     9
    10
    11SUMMARY: 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:

    0$ src/bitcoin-cli prioritisetransaction cafebabecafebabecafebabecafebabecafebabecafebabecafebabecafebabe 0 -9123456789123456789
    1true
    2$ src/bitcoin-cli prioritisetransaction cafebabecafebabecafebabecafebabecafebabecafebabecafebabecafebabe 0 -9123456789123456789
    3true
    

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

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

    or:

    0txid = wallet.send_self_transfer(from_node=self.nodes[0])['txid']
    1self.nodes[0].prioritisetransaction(txid='cafebabecafebabecafebabecafebabecafebabecafebabecafebabecafebabe', fee_delta=-9123456789123456789)
    2self.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?

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

    Reproduced on current master @ 7b975639ef93b50537a3ec6326b54d7218afc8da

    0$ clang --version
    1clang version 9.0.1-15+b1 
    2Target: x86_64-pc-linux-gnu
    3Thread model: posix
    4InstalledDir: /usr/bin
    
     02021-01-12T23:52:08Z PrioritiseTransaction: cafebabecafebabecafebabecafebabecafebabecafebabecafebabecafebabe feerate += -91234567891.23456789
     1txmempool.cpp:832:15: runtime error: signed integer overflow: -9123456789123456789 + -9123456789123456789 cannot be represented in type 'long'
     2    [#0](/bitcoin-bitcoin/0/) 0x5624c3cb287c in CTxMemPool::PrioritiseTransaction(uint256 const&, long const&) /home/jon/projects/bitcoin/bitcoin/src/txmempool.cpp:832:15
     3    [#1](/bitcoin-bitcoin/1/) 0x5624c3a69eb8 in prioritisetransaction()::$_6::operator()(RPCHelpMan const&, JSONRPCRequest const&) const /home/jon/projects/bitcoin/bitcoin/src/rpc/mining.cpp:470:36
     4    [#2](/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
     5    [#3](/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
     6    [#4](/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
     7    [#5](/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
     8    [#6](/bitcoin-bitcoin/6/) 0x5624c3965f88 in RPCHelpMan::HandleRequest(JSONRPCRequest const&) /home/jon/projects/bitcoin/bitcoin/src/./rpc/util.h:342:16
     9    [#7](/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
    10    [#8](/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
    11    [#9](/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
    12    [#10](/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
    13    [#11](/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
    14    [#12](/bitcoin-bitcoin/12/) 0x5624c3ba61ec in ExecuteCommand(CRPCCommand const&, JSONRPCRequest const&, UniValue&, bool) /home/jon/projects/bitcoin/bitcoin/src/rpc/server.cpp:466:20
    15    [#13](/bitcoin-bitcoin/13/) 0x5624c3ba5ac0 in CRPCTable::execute(JSONRPCRequest const&) const /home/jon/projects/bitcoin/bitcoin/src/rpc/server.cpp:449:17
    16    [#14](/bitcoin-bitcoin/14/) 0x5624c3ef9471 in HTTPReq_JSONRPC(util::Ref const&, HTTPRequest*) /home/jon/projects/bitcoin/bitcoin/src/httprpc.cpp:201:40
    17    [#15](/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
    18    [#16](/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
    19    [#17](/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
    20    [#18](/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
    21    [#19](/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
    22    [#20](/bitcoin-bitcoin/20/) 0x5624c3f1d258 in HTTPWorkItem::operator()() /home/jon/projects/bitcoin/bitcoin/src/httpserver.cpp:49:9
    23    [#21](/bitcoin-bitcoin/21/) 0x5624c3f27cbc in WorkQueue<HTTPClosure>::Run() /home/jon/projects/bitcoin/bitcoin/src/httpserver.cpp:108:13
    24    [#22](/bitcoin-bitcoin/22/) 0x5624c3f0deb3 in HTTPWorkQueueRun(WorkQueue<HTTPClosure>*, int) /home/jon/projects/bitcoin/bitcoin/src/httpserver.cpp:336:12
    25    [#23](/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
    26    [#24](/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
    27    [#25](/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
    28    [#26](/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
    29    [#27](/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
    30    [#28](/bitcoin-bitcoin/28/) 0x7fc51f0c9ecf  (/lib/x86_64-linux-gnu/libstdc++.so.6+0xceecf)
    31    [#29](/bitcoin-bitcoin/29/) 0x7fc51f34fea6 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x8ea6)
    32    [#30](/bitcoin-bitcoin/30/) 0x7fc51edc2dee in clone (/lib/x86_64-linux-gnu/libc.so.6+0xfddee)
    33
    34SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior txmempool.cpp:832:15 in 
    352021-01-12T23:52:19Z PrioritiseTransaction: cafebabecafebabecafebabecafebabecafebabecafebabecafebabecafebabe feerate += -91234567891.23456789
    
  5. MarcoFalke commented at 9:57 am on January 14, 2021: member
    Fixed in #20383
  6. MarcoFalke referenced this in commit 824eea5643 on Apr 5, 2021
  7. MarcoFalke commented at 1:49 pm on April 6, 2021: member
    Looks like fuzzing with -ftrapv will currently fail due to this
  8. MarcoFalke commented at 10:33 am on May 10, 2021: member
  9. practicalswift closed this on Oct 28, 2021

  10. MarcoFalke referenced this in commit dde7205c57 on Jun 27, 2022
  11. sidhujag referenced this in commit 758481e22d on Jun 27, 2022
  12. DrahtBot 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: 2024-07-05 22:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me