diff options
-rw-r--r-- | doc/release-notes-28113.md | 7 | ||||
-rw-r--r-- | src/Makefile.am | 4 | ||||
-rw-r--r-- | src/bitcoin-tx.cpp | 10 | ||||
-rw-r--r-- | src/core_io.h | 4 | ||||
-rw-r--r-- | src/core_read.cpp | 47 | ||||
-rw-r--r-- | src/rpc/util.cpp | 18 | ||||
-rw-r--r-- | src/rpc/util.h | 3 | ||||
-rw-r--r-- | src/test/fuzz/parse_univalue.cpp | 9 |
8 files changed, 59 insertions, 43 deletions
diff --git a/doc/release-notes-28113.md b/doc/release-notes-28113.md new file mode 100644 index 0000000000..c1a3a07b17 --- /dev/null +++ b/doc/release-notes-28113.md @@ -0,0 +1,7 @@ +RPC Wallet +---------- + +- The `signrawtransactionwithkey`, `signrawtransactionwithwallet`, + `walletprocesspsbt` and `descriptorprocesspsbt` calls now return more + specific RPC_INVALID_PARAMETER instead of RPC_PARSE_ERROR if their + sighashtype argument is malformed or not a string. diff --git a/src/Makefile.am b/src/Makefile.am index aba6f00756..dfea7146aa 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -898,8 +898,8 @@ if BUILD_BITCOIN_KERNEL_LIB lib_LTLIBRARIES += $(LIBBITCOINKERNEL) libbitcoinkernel_la_LDFLAGS = $(AM_LDFLAGS) -no-undefined $(RELDFLAGS) $(PTHREAD_FLAGS) -libbitcoinkernel_la_LIBADD = $(LIBBITCOIN_CRYPTO) $(LIBUNIVALUE) $(LIBLEVELDB) $(LIBMEMENV) $(LIBSECP256K1) -libbitcoinkernel_la_CPPFLAGS = $(AM_CPPFLAGS) -I$(builddir)/obj -I$(srcdir)/secp256k1/include -DBUILD_BITCOIN_INTERNAL $(BOOST_CPPFLAGS) $(LEVELDB_CPPFLAGS) -I$(srcdir)/$(UNIVALUE_INCLUDE_DIR_INT) +libbitcoinkernel_la_LIBADD = $(LIBBITCOIN_CRYPTO) $(LIBLEVELDB) $(LIBMEMENV) $(LIBSECP256K1) +libbitcoinkernel_la_CPPFLAGS = $(AM_CPPFLAGS) -I$(builddir)/obj -I$(srcdir)/secp256k1/include -DBUILD_BITCOIN_INTERNAL $(BOOST_CPPFLAGS) $(LEVELDB_CPPFLAGS) # libbitcoinkernel requires default symbol visibility, explicitly specify that # here so that things still work even when user configures with diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index 0c25ddf373..103d8885db 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -562,6 +562,16 @@ static CAmount AmountFromValue(const UniValue& value) return amount; } +static std::vector<unsigned char> ParseHexUV(const UniValue& v, const std::string& strName) +{ + std::string strHex; + if (v.isStr()) + strHex = v.getValStr(); + if (!IsHex(strHex)) + throw std::runtime_error(strName + " must be hexadecimal string (not '" + strHex + "')"); + return ParseHex(strHex); +} + static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr) { int nHashType = SIGHASH_ALL; diff --git a/src/core_io.h b/src/core_io.h index 997f3bfd5b..1f5ecbaea6 100644 --- a/src/core_io.h +++ b/src/core_io.h @@ -6,6 +6,7 @@ #define BITCOIN_CORE_IO_H #include <consensus/amount.h> +#include <util/result.h> #include <string> #include <vector> @@ -45,8 +46,7 @@ bool DecodeHexBlockHeader(CBlockHeader&, const std::string& hex_header); * @see ParseHashV for an RPC-oriented version of this */ bool ParseHashStr(const std::string& strHex, uint256& result); -std::vector<unsigned char> ParseHexUV(const UniValue& v, const std::string& strName); -int ParseSighashString(const UniValue& sighash); +[[nodiscard]] util::Result<int> SighashFromStr(const std::string& sighash); // core_write.cpp UniValue ValueFromAmount(const CAmount amount); diff --git a/src/core_read.cpp b/src/core_read.cpp index 84cd559b7f..dfabf3a0c2 100644 --- a/src/core_read.cpp +++ b/src/core_read.cpp @@ -10,7 +10,7 @@ #include <script/sign.h> #include <serialize.h> #include <streams.h> -#include <univalue.h> +#include <util/result.h> #include <util/strencodings.h> #include <version.h> @@ -242,36 +242,21 @@ bool ParseHashStr(const std::string& strHex, uint256& result) return true; } -std::vector<unsigned char> ParseHexUV(const UniValue& v, const std::string& strName) +util::Result<int> SighashFromStr(const std::string& sighash) { - std::string strHex; - if (v.isStr()) - strHex = v.getValStr(); - if (!IsHex(strHex)) - throw std::runtime_error(strName + " must be hexadecimal string (not '" + strHex + "')"); - return ParseHex(strHex); -} - -int ParseSighashString(const UniValue& sighash) -{ - int hash_type = SIGHASH_DEFAULT; - if (!sighash.isNull()) { - static std::map<std::string, int> map_sighash_values = { - {std::string("DEFAULT"), int(SIGHASH_DEFAULT)}, - {std::string("ALL"), int(SIGHASH_ALL)}, - {std::string("ALL|ANYONECANPAY"), int(SIGHASH_ALL|SIGHASH_ANYONECANPAY)}, - {std::string("NONE"), int(SIGHASH_NONE)}, - {std::string("NONE|ANYONECANPAY"), int(SIGHASH_NONE|SIGHASH_ANYONECANPAY)}, - {std::string("SINGLE"), int(SIGHASH_SINGLE)}, - {std::string("SINGLE|ANYONECANPAY"), int(SIGHASH_SINGLE|SIGHASH_ANYONECANPAY)}, - }; - const std::string& strHashType = sighash.get_str(); - const auto& it = map_sighash_values.find(strHashType); - if (it != map_sighash_values.end()) { - hash_type = it->second; - } else { - throw std::runtime_error(strHashType + " is not a valid sighash parameter."); - } + static std::map<std::string, int> map_sighash_values = { + {std::string("DEFAULT"), int(SIGHASH_DEFAULT)}, + {std::string("ALL"), int(SIGHASH_ALL)}, + {std::string("ALL|ANYONECANPAY"), int(SIGHASH_ALL|SIGHASH_ANYONECANPAY)}, + {std::string("NONE"), int(SIGHASH_NONE)}, + {std::string("NONE|ANYONECANPAY"), int(SIGHASH_NONE|SIGHASH_ANYONECANPAY)}, + {std::string("SINGLE"), int(SIGHASH_SINGLE)}, + {std::string("SINGLE|ANYONECANPAY"), int(SIGHASH_SINGLE|SIGHASH_ANYONECANPAY)}, + }; + const auto& it = map_sighash_values.find(sighash); + if (it != map_sighash_values.end()) { + return it->second; + } else { + return util::Error{Untranslated(sighash + " is not a valid sighash parameter.")}; } - return hash_type; } diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 19e14f88df..377181dd81 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -3,8 +3,10 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <clientversion.h> +#include <core_io.h> #include <common/args.h> #include <consensus/amount.h> +#include <script/interpreter.h> #include <key_io.h> #include <outputtype.h> #include <rpc/util.h> @@ -12,6 +14,7 @@ #include <script/signingprovider.h> #include <tinyformat.h> #include <util/check.h> +#include <util/result.h> #include <util/strencodings.h> #include <util/string.h> #include <util/translation.h> @@ -310,6 +313,21 @@ UniValue DescribeAddress(const CTxDestination& dest) return std::visit(DescribeAddressVisitor(), dest); } +int ParseSighashString(const UniValue& sighash) +{ + if (sighash.isNull()) { + return SIGHASH_DEFAULT; + } + if (!sighash.isStr()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "sighash needs to be null or string"); + } + const auto result{SighashFromStr(sighash.get_str())}; + if (!result) { + throw JSONRPCError(RPC_INVALID_PARAMETER, util::ErrorString(result).original); + } + return result.value(); +} + unsigned int ParseConfirmTarget(const UniValue& value, unsigned int max_target) { const int target{value.getInt<int>()}; diff --git a/src/rpc/util.h b/src/rpc/util.h index 4cba5a9818..02d26f1ab7 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -100,6 +100,9 @@ CTxDestination AddAndGetMultisigDestination(const int required, const std::vecto UniValue DescribeAddress(const CTxDestination& dest); +/** Parse a sighash string representation and raise an RPC error if it is invalid. */ +int ParseSighashString(const UniValue& sighash); + //! Parse a confirm target option and raise an RPC error if it is invalid. unsigned int ParseConfirmTarget(const UniValue& value, unsigned int max_target); diff --git a/src/test/fuzz/parse_univalue.cpp b/src/test/fuzz/parse_univalue.cpp index bfa856211d..c9096d0386 100644 --- a/src/test/fuzz/parse_univalue.cpp +++ b/src/test/fuzz/parse_univalue.cpp @@ -3,7 +3,6 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <chainparams.h> -#include <core_io.h> #include <rpc/client.h> #include <rpc/util.h> #include <test/fuzz/fuzz.h> @@ -58,12 +57,6 @@ FUZZ_TARGET(parse_univalue, .init = initialize_parse_univalue) } catch (const UniValue&) { } try { - (void)ParseHexUV(univalue, "A"); - (void)ParseHexUV(univalue, random_string); - } catch (const UniValue&) { - } catch (const std::runtime_error&) { - } - try { (void)ParseHexV(univalue, "A"); } catch (const UniValue&) { } catch (const std::runtime_error&) { @@ -75,7 +68,7 @@ FUZZ_TARGET(parse_univalue, .init = initialize_parse_univalue) } try { (void)ParseSighashString(univalue); - } catch (const std::runtime_error&) { + } catch (const UniValue&) { } try { (void)AmountFromValue(univalue); |