From 510c6532bae9abc5beda1c126c945923a64680cb Mon Sep 17 00:00:00 2001 From: Ben Woosley Date: Thu, 4 Apr 2019 00:39:04 -0700 Subject: Extract ParseDescriptorRange So as to be consistently informative when the checks fail, and to protect against unintentional divergence among the checks. --- src/rpc/blockchain.cpp | 3 +-- src/rpc/misc.cpp | 14 ++------------ src/rpc/util.cpp | 20 +++++++++++++++++++- src/rpc/util.h | 2 +- src/wallet/rpcdump.cpp | 8 ++------ test/functional/rpc_scantxoutset.py | 9 ++++++++- test/functional/wallet_importmulti.py | 15 +++++++++++++++ 7 files changed, 48 insertions(+), 23 deletions(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index d35f458b2e..53f8dfebaa 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -2224,8 +2224,7 @@ UniValue scantxoutset(const JSONRPCRequest& request) desc_str = desc_uni.get_str(); UniValue range_uni = find_value(scanobject, "range"); if (!range_uni.isNull()) { - range = ParseRange(range_uni); - if (range.first < 0 || (range.second >> 31) != 0 || range.second >= range.first + 1000000) throw JSONRPCError(RPC_INVALID_PARAMETER, "range out of range"); + range = ParseDescriptorRange(range_uni); } } else { throw JSONRPCError(RPC_INVALID_PARAMETER, "Scan object needs to be either a string or an object"); diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index 0a97f80297..e07b02aa23 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -23,6 +23,7 @@ #include #include +#include #ifdef HAVE_MALLOC_INFO #include #endif @@ -214,18 +215,7 @@ UniValue deriveaddresses(const JSONRPCRequest& request) int64_t range_end = 0; if (request.params.size() >= 2 && !request.params[1].isNull()) { - auto range = ParseRange(request.params[1]); - if (range.first < 0) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Range should be greater or equal than 0"); - } - if ((range.second >> 31) != 0) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "End of range is too high"); - } - if (range.second >= range.first + 1000000) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Range is too large"); - } - range_begin = range.first; - range_end = range.second; + std::tie(range_begin, range_end) = ParseDescriptorRange(request.params[1]); } FlatSigningProvider provider; diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 10979b43b0..8f8488bc5f 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -8,6 +8,8 @@ #include #include +#include + InitInterfaces* g_rpc_interfaces = nullptr; // Converts a hex string to a public key if possible @@ -529,7 +531,7 @@ std::string RPCArg::ToString(const bool oneline) const assert(false); } -std::pair ParseRange(const UniValue& value) +static std::pair ParseRange(const UniValue& value) { if (value.isNum()) { return {0, value.get_int64()}; @@ -542,3 +544,19 @@ std::pair ParseRange(const UniValue& value) } throw JSONRPCError(RPC_INVALID_PARAMETER, "Range must be specified as end or as [begin,end]"); } + +std::pair ParseDescriptorRange(const UniValue& value) +{ + int64_t low, high; + std::tie(low, high) = ParseRange(value); + if (low < 0) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Range should be greater or equal than 0"); + } + if ((high >> 31) != 0) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "End of range is too high"); + } + if (high >= low + 1000000) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Range is too large"); + } + return {low, high}; +} diff --git a/src/rpc/util.h b/src/rpc/util.h index e4cc1fde44..d2edf30e02 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -39,7 +39,7 @@ RPCErrorCode RPCErrorFromTransactionError(TransactionError terr); UniValue JSONRPCTransactionError(TransactionError terr, const std::string& err_string = ""); //! Parse a JSON range specified as int64, or [int64, int64] -std::pair ParseRange(const UniValue& value); +std::pair ParseDescriptorRange(const UniValue& value); struct RPCArg { enum class Type { diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 9c5dae3623..674b56fe9d 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -22,6 +22,7 @@ #include #include +#include #include #include @@ -1144,12 +1145,7 @@ static UniValue ProcessImportDescriptor(ImportData& import_data, std::map> 31) != 0 || range_end - range_start >= 1000000) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid descriptor range specified"); - } + std::tie(range_start, range_end) = ParseDescriptorRange(data["range"]); } const UniValue& priv_keys = data.exists("keys") ? data["keys"].get_array() : UniValue(); diff --git a/test/functional/rpc_scantxoutset.py b/test/functional/rpc_scantxoutset.py index 6346477922..a1cd33ad54 100755 --- a/test/functional/rpc_scantxoutset.py +++ b/test/functional/rpc_scantxoutset.py @@ -4,7 +4,7 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test the scantxoutset rpc call.""" from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal +from test_framework.util import assert_equal, assert_raises_rpc_error from decimal import Decimal import shutil @@ -67,6 +67,13 @@ class ScantxoutsetTest(BitcoinTestFramework): assert_equal(self.nodes[0].scantxoutset("start", [ "addr(" + addr_P2SH_SEGWIT + ")", "addr(" + addr_LEGACY + ")", "addr(" + addr_BECH32 + ")"])['total_amount'], Decimal("0.007")) assert_equal(self.nodes[0].scantxoutset("start", [ "addr(" + addr_P2SH_SEGWIT + ")", "addr(" + addr_LEGACY + ")", "combo(" + pubk3 + ")"])['total_amount'], Decimal("0.007")) + self.log.info("Test range validation.") + assert_raises_rpc_error(-8, "End of range is too high", self.nodes[0].scantxoutset, "start", [ {"desc": "desc", "range": -1}]) + assert_raises_rpc_error(-8, "Range should be greater or equal than 0", self.nodes[0].scantxoutset, "start", [ {"desc": "desc", "range": [-1, 10]}]) + assert_raises_rpc_error(-8, "End of range is too high", self.nodes[0].scantxoutset, "start", [ {"desc": "desc", "range": [(2 << 31 + 1) - 1000000, (2 << 31 + 1)]}]) + assert_raises_rpc_error(-8, "Range specified as [begin,end] must not have begin after end", self.nodes[0].scantxoutset, "start", [ {"desc": "desc", "range": [2, 1]}]) + assert_raises_rpc_error(-8, "Range is too large", self.nodes[0].scantxoutset, "start", [ {"desc": "desc", "range": [0, 1000001]}]) + self.log.info("Test extended key derivation.") # Run various scans, and verify that the sum of the amounts of the matches corresponds to the expected subset. # Note that all amounts in the UTXO set are powers of 2 multiplied by 0.001 BTC, so each amounts uniquely identifies a subset. diff --git a/test/functional/wallet_importmulti.py b/test/functional/wallet_importmulti.py index 5bfbaa2f0b..939390ecfe 100755 --- a/test/functional/wallet_importmulti.py +++ b/test/functional/wallet_importmulti.py @@ -591,6 +591,21 @@ class ImportMultiTest(BitcoinTestFramework): key.p2sh_p2wpkh_addr, solvable=True) + self.test_importmulti({"desc": descsum_create(desc), "timestamp": "now", "range": -1}, + success=False, error_code=-8, error_message='End of range is too high') + + self.test_importmulti({"desc": descsum_create(desc), "timestamp": "now", "range": [-1, 10]}, + success=False, error_code=-8, error_message='Range should be greater or equal than 0') + + self.test_importmulti({"desc": descsum_create(desc), "timestamp": "now", "range": [(2 << 31 + 1) - 1000000, (2 << 31 + 1)]}, + success=False, error_code=-8, error_message='End of range is too high') + + self.test_importmulti({"desc": descsum_create(desc), "timestamp": "now", "range": [2, 1]}, + success=False, error_code=-8, error_message='Range specified as [begin,end] must not have begin after end') + + self.test_importmulti({"desc": descsum_create(desc), "timestamp": "now", "range": [0, 1000001]}, + success=False, error_code=-8, error_message='Range is too large') + # Test importing of a P2PKH address via descriptor key = get_key(self.nodes[0]) self.log.info("Should import a p2pkh address from descriptor") -- cgit v1.2.3