wallet: Uninitialized read in bumpfee(…) #17642
Description
Uninitialized read in bumpfee(…).
The problem can be verified by running test/functional/wallet_bumpfee.py --valgrind (see PR #17633 for --valgrind).
Live demo:
$ test/functional/wallet_bumpfee.py --valgrind --tracerpc
2019-11-30T20:58:24.457000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_x66swmkm
…
-152-> bumpfee ["b8f5472384ca8f1b69c64f058db13d545e3d0b82aec4e777a77087830159ef11", {"fee_rate": 0.0015}]
2019-11-30T21:00:18.358000Z TestFramework (ERROR): Unexpected exception caught during testing
…
ConnectionRefusedError: [Errno 111] Connection refused
$ cat /tmp/bitcoin_func_test_x66swmkm/node1/stderr/*
==17181== Thread 15 b-httpworker.0:
==17181== Conditional jump or move depends on uninitialised value(s)
==17181== at 0x8F00BC: ValueFromAmount(long const&) (core_write.cpp:21)
==17181== by 0x76FA48: bumpfee(JSONRPCRequest const&) (rpcwallet.cpp:3482)
==17181== by 0x375CE2: CRPCCommand::CRPCCommand(char const*, char const*, UniValue (*)(JSONRPCRequest const&), std::initializer_list<char const*>)::{lambda(JSONRPCRequest const&, UniValue&, bool)#1}::operator()(JSONRPCRequest const&, UniValue&, bool) const (server.h:104)
==17181== by 0x375AE0: std::_Function_handler<bool (JSONRPCRequest const&, UniValue&, bool), CRPCCommand::CRPCCommand(char const*, char const*, UniValue (*)(JSONRPCRequest const&), std::initializer_list<char const*>)::{lambda(JSONRPCRequest const&, UniValue&, bool)#1}>::_M_invoke(std::_Any_data const&, JSONRPCRequest const&, UniValue&, bool&&) (std_function.h:282)
==17181== by 0x16E0E0: std::function<bool (JSONRPCRequest const&, UniValue&, bool)>::operator()(JSONRPCRequest const&, UniValue&, bool) const (std_function.h:687)
==17181== by 0x165D3E: interfaces::(anonymous namespace)::RpcHandlerImpl::RpcHandlerImpl(CRPCCommand const&)::{lambda(JSONRPCRequest const&, UniValue&, bool)#1}::operator()(JSONRPCRequest const&, UniValue&, bool) const (chain.cpp:202)
==17181== by 0x165B00: std::_Function_handler<bool (JSONRPCRequest const&, UniValue&, bool), interfaces::(anonymous namespace)::RpcHandlerImpl::RpcHandlerImpl(CRPCCommand const&)::{lambda(JSONRPCRequest const&, UniValue&, bool)#1}>::_M_invoke(std::_Any_data const&, JSONRPCRequest const&, UniValue&, bool&&) (std_function.h:282)
==17181== by 0x16E0E0: std::function<bool (JSONRPCRequest const&, UniValue&, bool)>::operator()(JSONRPCRequest const&, UniValue&, bool) const (std_function.h:687)
==17181== by 0x41FF47: ExecuteCommand(CRPCCommand const&, JSONRPCRequest const&, UniValue&, bool) (server.cpp:449)
==17181== by 0x41FBC2: CRPCTable::execute(JSONRPCRequest const&) const (server.cpp:432)
==17181== by 0x67771B: HTTPReq_JSONRPC(HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (httprpc.cpp:190)
==17181== by 0x336249: std::_Function_handler<bool (HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&), bool (*)(HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)>::_M_invoke(std::_Any_data const&, HTTPRequest*&&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (std_function.h:282)
…
The following diff avoids the uninitialized read but is not a correct fix:
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
index d906f6ddf072..d09d08f05480 100644
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@@ -3437,7 +3437,7 @@ static UniValue bumpfee(const JSONRPCRequest& request)
std::vector<std::string> errors;
- CAmount old_fee;
+ CAmount old_fee = -1;
CAmount new_fee;
CMutableTransaction mtx;
feebumper::Result res;
@@ -3479,7 +3479,9 @@ static UniValue bumpfee(const JSONRPCRequest& request)
}
UniValue result(UniValue::VOBJ);
result.pushKV("txid", txid.GetHex());
- result.pushKV("origfee", ValueFromAmount(old_fee));
+ if (MoneyRange(old_fee)) {
+ result.pushKV("origfee", ValueFromAmount(old_fee));
+ }
result.pushKV("fee", ValueFromAmount(new_fee));
UniValue result_errors(UniValue::VARR);
for (const std::string& error : errors) {