diff --git a/src/core_write.cpp b/src/core_write.cpp index 253dfde1006..c9fded11136 100644 --- a/src/core_write.cpp +++ b/src/core_write.cpp @@ -153,7 +153,9 @@ void ScriptToUniv(const CScript& script, UniValue& out, bool include_hex, bool i out.pushKV("asm", ScriptToAsmStr(script)); if (include_address) { - out.pushKV("desc", InferDescriptor(script, provider ? *provider : DUMMY_SIGNING_PROVIDER)->ToString()); + auto desc = InferDescriptor(script, provider ? *provider : DUMMY_SIGNING_PROVIDER); + // future: make 'InferDescriptor' return the error cause + if (desc) out.pushKV("desc", desc->ToString()); } if (include_hex) { out.pushKV("hex", HexStr(script)); diff --git a/src/policy/policy.cpp b/src/policy/policy.cpp index ed336928235..ebd999ef1b8 100644 --- a/src/policy/policy.cpp +++ b/src/policy/policy.cpp @@ -87,7 +87,7 @@ bool IsStandard(const CScript& scriptPubKey, const std::optional& max_ unsigned char m = vSolutions.front()[0]; unsigned char n = vSolutions.back()[0]; // Support up to x-of-3 multisig txns as standard - if (n < 1 || n > 3) + if (n < 1 || n > MAX_BARE_MULTISIG_PUBKEYS_NUM) return false; if (m < 1 || m > n) return false; diff --git a/src/policy/policy.h b/src/policy/policy.h index 2151ec13dd0..ccb535b69cb 100644 --- a/src/policy/policy.h +++ b/src/policy/policy.h @@ -44,6 +44,8 @@ static constexpr unsigned int DEFAULT_INCREMENTAL_RELAY_FEE{1000}; static constexpr unsigned int DEFAULT_BYTES_PER_SIGOP{20}; /** Default for -permitbaremultisig */ static constexpr bool DEFAULT_PERMIT_BAREMULTISIG{true}; +/** The maximum number of pubkeys in a bare multisig output script */ +static constexpr unsigned int MAX_BARE_MULTISIG_PUBKEYS_NUM{3}; /** The maximum number of witness stack items in a standard P2WSH script */ static constexpr unsigned int MAX_STANDARD_P2WSH_STACK_ITEMS{100}; /** The maximum size in bytes of each witness stack item in a standard P2WSH script */ diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index ac1ce6285f7..c3b94f388ac 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1108,7 +1108,7 @@ static RPCHelpMan gettxout() {RPCResult::Type::STR_AMOUNT, "value", "The transaction value in " + CURRENCY_UNIT}, {RPCResult::Type::OBJ, "scriptPubKey", "", { {RPCResult::Type::STR, "asm", "Disassembly of the output script"}, - {RPCResult::Type::STR, "desc", "Inferred descriptor for the output"}, + {RPCResult::Type::STR, "desc", /*optional=*/true, "Inferred top level descriptor for the script (if any)"}, {RPCResult::Type::STR_HEX, "hex", "The raw output script bytes, hex-encoded"}, {RPCResult::Type::STR, "type", "The type, eg pubkeyhash"}, {RPCResult::Type::STR, "address", /*optional=*/true, "The Bitcoin address (only if a well-defined address exists)"}, @@ -2300,9 +2300,10 @@ static RPCHelpMan scantxoutset() FlatSigningProvider provider; auto scripts = EvalDescriptorStringOrObject(scanobject, provider); for (CScript& script : scripts) { - std::string inferred = InferDescriptor(script, provider)->ToString(); + auto desc = InferDescriptor(script, provider); + if (!desc) throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid descriptor found: '%s'", scanobject.get_str())); needles.emplace(script); - descriptors.emplace(std::move(script), std::move(inferred)); + descriptors.emplace(std::move(script), desc->ToString()); } } diff --git a/src/rpc/output_script.cpp b/src/rpc/output_script.cpp index 5de2847be99..b32bccaf6ce 100644 --- a/src/rpc/output_script.cpp +++ b/src/rpc/output_script.cpp @@ -146,6 +146,7 @@ static RPCHelpMan createmultisig() // Make the descriptor std::unique_ptr descriptor = InferDescriptor(GetScriptForDestination(dest), keystore); + if (!descriptor) throw JSONRPCError(RPC_INTERNAL_ERROR, "Invalid descriptor generated"); UniValue result(UniValue::VOBJ); result.pushKV("address", EncodeDestination(dest)); diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 421656152cb..7bdd9c2fce7 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -493,7 +493,7 @@ static RPCHelpMan decodescript() RPCResult::Type::OBJ, "", "", { {RPCResult::Type::STR, "asm", "Disassembly of the script"}, - {RPCResult::Type::STR, "desc", "Inferred descriptor for the script"}, + {RPCResult::Type::STR, "desc", /*optional=*/true, "Inferred top level descriptor for the script (if any)"}, {RPCResult::Type::STR, "type", "The output type (e.g. " + GetAllOutputTypes() + ")"}, {RPCResult::Type::STR, "address", /*optional=*/true, "The Bitcoin address (only if a well-defined address exists)"}, {RPCResult::Type::STR, "p2sh", /*optional=*/true, @@ -505,7 +505,7 @@ static RPCHelpMan decodescript() {RPCResult::Type::STR_HEX, "hex", "The raw output script bytes, hex-encoded"}, {RPCResult::Type::STR, "type", "The type of the output script (e.g. witness_v0_keyhash or witness_v0_scripthash)"}, {RPCResult::Type::STR, "address", /*optional=*/true, "The Bitcoin address (only if a well-defined address exists)"}, - {RPCResult::Type::STR, "desc", "Inferred descriptor for the script"}, + {RPCResult::Type::STR, "desc", /*optional=*/true, "Inferred descriptor for the script (if any)"}, {RPCResult::Type::STR, "p2sh-segwit", "address of the P2SH script wrapping this witness redeem script"}, }}, }, @@ -529,9 +529,14 @@ static RPCHelpMan decodescript() std::vector> solutions_data; const TxoutType which_type{Solver(script, solutions_data)}; + bool can_wrap_p2sh = true; const bool can_wrap{[&] { switch (which_type) { - case TxoutType::MULTISIG: + case TxoutType::MULTISIG: { + // Verify it doesn't exceed p2sh consensus limits + can_wrap_p2sh = script.size() <= MAX_SCRIPT_ELEMENT_SIZE; + break; + } case TxoutType::NONSTANDARD: case TxoutType::PUBKEY: case TxoutType::PUBKEYHASH: @@ -561,7 +566,7 @@ static RPCHelpMan decodescript() }()}; if (can_wrap) { - r.pushKV("p2sh", EncodeDestination(ScriptHash(script))); + if (can_wrap_p2sh) r.pushKV("p2sh", EncodeDestination(ScriptHash(script))); // P2SH and witness programs cannot be wrapped in P2WSH, if this script // is a witness program, don't return addresses for a segwit programs. const bool can_wrap_P2WSH{[&] { @@ -822,7 +827,7 @@ const RPCResult decodepsbt_inputs{ {RPCResult::Type::OBJ, "scriptPubKey", "", { {RPCResult::Type::STR, "asm", "Disassembly of the output script"}, - {RPCResult::Type::STR, "desc", "Inferred descriptor for the output"}, + {RPCResult::Type::STR, "desc", /*optional=*/true, "Inferred top level descriptor for the script (if any)"}, {RPCResult::Type::STR_HEX, "hex", "The raw output script bytes, hex-encoded"}, {RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"}, {RPCResult::Type::STR, "address", /*optional=*/true, "The Bitcoin address (only if a well-defined address exists)"}, diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index c8802d2bf89..a8088ee9971 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -1841,8 +1841,8 @@ std::vector> ParseScript(uint32_t& key_exp_index return {}; } if (ctx == ParseScriptContext::TOP) { - if (providers.size() > 3) { - error = strprintf("Cannot have %u pubkeys in bare multisig; only at most 3 pubkeys", providers.size()); + if (providers.size() > MAX_BARE_MULTISIG_PUBKEYS_NUM) { + error = strprintf("Cannot have %u pubkeys in bare multisig; only at most %u pubkeys", providers.size(), MAX_BARE_MULTISIG_PUBKEYS_NUM); return {}; } } @@ -2228,6 +2228,17 @@ std::unique_ptr InferScript(const CScript& script, ParseScriptCo break; } } + + // Check bare multisig pubkeys number limit + if (ctx == ParseScriptContext::TOP && providers.size() > MAX_BARE_MULTISIG_PUBKEYS_NUM) { + return nullptr; + } + + // Verify we don't exceed consensus limits + if (ctx == ParseScriptContext::P2SH && script.size() > MAX_SCRIPT_ELEMENT_SIZE) { + return nullptr; + } + if (ok) return std::make_unique((int)data[0][0], std::move(providers)); } if (txntype == TxoutType::SCRIPTHASH && ctx == ParseScriptContext::TOP) { diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp index 1c2951deeec..8b9d6da7b24 100644 --- a/src/wallet/rpc/addresses.cpp +++ b/src/wallet/rpc/addresses.cpp @@ -292,6 +292,13 @@ RPCHelpMan addmultisigaddress() CScript inner; CTxDestination dest = AddAndGetMultisigDestination(required, pubkeys, output_type, provider, inner); + // Before importing the scripts into the wallet, check we can make a descriptor out of it. + std::unique_ptr descriptor = InferDescriptor(GetScriptForDestination(dest), spk_man); + if (!descriptor) { + // Shouldn't happen; we crafted the destination internally. + throw JSONRPCError(RPC_INTERNAL_ERROR, "Invalid descriptor generated"); + } + // Import scripts into the wallet for (const auto& [id, script] : provider.scripts) { // Due to a bug in the legacy wallet, the p2sh maximum script size limit is also imposed on 'p2sh-segwit' and 'bech32' redeem scripts. @@ -313,9 +320,6 @@ RPCHelpMan addmultisigaddress() // Store destination in the addressbook pwallet->SetAddressBook(dest, label, AddressPurpose::SEND); - // Make the descriptor - std::unique_ptr descriptor = InferDescriptor(GetScriptForDestination(dest), spk_man); - UniValue result(UniValue::VOBJ); result.pushKV("address", EncodeDestination(dest)); result.pushKV("redeemScript", HexStr(inner)); diff --git a/test/functional/rpc_decodescript.py b/test/functional/rpc_decodescript.py index 4f127fa8b7f..507b4628a62 100755 --- a/test/functional/rpc_decodescript.py +++ b/test/functional/rpc_decodescript.py @@ -96,6 +96,16 @@ class DecodeScriptTest(BitcoinTestFramework): assert_equal('witness_v0_scripthash', rpc_result['segwit']['type']) assert_equal('0 ' + multisig_script_hash, rpc_result['segwit']['asm']) + # Check decoding a multisig redeem script doesn't return an invalid descriptor + self.log.info("- multisig invalid") + # Decode a +520 bytes consensus-invalid p2sh multisig - provided script translates to multi(20, keys) + large_multisig_script = '0114' + push_public_key * 20 + '0114' + 'ae' + rpc_result = self.nodes[0].decodescript(large_multisig_script) + # Verify it is valid only under segwit rules, not for bare multisig nor p2sh. + assert 'desc' not in rpc_result + assert 'p2sh' not in rpc_result + assert 'segwit' in rpc_result + self.log.info ("- P2SH") # OP_HASH160 OP_EQUAL. # push_public_key_hash here should actually be the hash of a redeem script.