diff --git a/doc/release-notes-31603.md b/doc/release-notes-31603.md new file mode 100644 index 00000000000..81923721140 --- /dev/null +++ b/doc/release-notes-31603.md @@ -0,0 +1,6 @@ +Updated RPCs +-------- + +- Any RPC in which one of the parameters are descriptors will throw an error +if the provided descriptor contains a whitespace at the beginning or the end +of the public key within a fragment - e.g. `pk( KEY)` or `pk(KEY )`. diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index c8802d2bf89..7552bfdf981 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -1509,6 +1509,10 @@ std::vector> ParsePubkeyInner(uint32_t key_exp_i error = "No key provided"; return {}; } + if (IsSpace(str.front()) || IsSpace(str.back())) { + error = strprintf("Key '%s' is invalid due to whitespace", str); + return {}; + } if (split.size() == 1) { if (IsHex(str)) { std::vector data = ParseHex(str); diff --git a/src/test/descriptor_tests.cpp b/src/test/descriptor_tests.cpp index 63c53a842c1..c68ab25696e 100644 --- a/src/test/descriptor_tests.cpp +++ b/src/test/descriptor_tests.cpp @@ -938,6 +938,11 @@ BOOST_AUTO_TEST_CASE(descriptor_test) CheckUnparsable("sh(sh(pk(L4rK1yDtCWekvXuE6oXD9jCYfFNV2cWRpVuPLBcCU2z8TrisoyY1)))", "sh(sh(pk(03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd)))", "Can only have sh() at top level"); // Cannot embed P2SH inside P2SH CheckUnparsable("wsh(wsh(pk(L4rK1yDtCWekvXuE6oXD9jCYfFNV2cWRpVuPLBcCU2z8TrisoyY1)))", "wsh(wsh(pk(03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd)))", "Can only have wsh() at top level or inside sh()"); // Cannot embed P2WSH inside P2WSH + // Check for whitespace into keys + CheckUnparsable("", "multi(1, L4rK1yDtCWekvXuE6oXD9jCYfFNV2cWRpVuPLBcCU2z8TrisoyY1,5KYZdUEo39z3FPrtuX2QbbwGnNP5zTd7yyr2SC1j299sBCnWjss)", "Multi: Key ' L4rK1yDtCWekvXuE6oXD9jCYfFNV2cWRpVuPLBcCU2z8TrisoyY1' is invalid due to whitespace"); + CheckUnparsable("", "pk(L4rK1yDtCWekvXuE6oXD9jCYfFNV2cWRpVuPLBcCU2z8TrisoyY1 )", "pk(): Key 'L4rK1yDtCWekvXuE6oXD9jCYfFNV2cWRpVuPLBcCU2z8TrisoyY1 ' is invalid due to whitespace"); + CheckUnparsable("", "pk( L4rK1yDtCWekvXuE6oXD9jCYfFNV2cWRpVuPLBcCU2z8TrisoyY1 )", "pk(): Key ' L4rK1yDtCWekvXuE6oXD9jCYfFNV2cWRpVuPLBcCU2z8TrisoyY1 ' is invalid due to whitespace"); + // Checksums Check("sh(multi(2,[00000000/111'/222]xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc,xprv9uPDJpEQgRQfDcW7BkF7eTya6RPxXeJCqCJGHuCJ4GiRVLzkTXBAJMu2qaMWPrS7AANYqdq6vcBcBUdJCVVFceUvJFjaPdGZ2y9WACViL4L/0))#ggrsrxfy", "sh(multi(2,[00000000/111'/222]xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y/0))#tjg09x5t", "sh(multi(2,[00000000/111h/222]xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y/0))#hgmsckna", DEFAULT, {{"a91445a9a622a8b0a1269944be477640eedc447bbd8487"}}, OutputType::LEGACY, /*op_desc_id=*/uint256{"9339b7bfbe8cfd9d0d55819778ef77f52e5786e85b4c83be8a0d5b976e033f4c"}, {{0x8000006FUL,222},{0}}); Check("sh(multi(2,[00000000/111'/222]xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc,xprv9uPDJpEQgRQfDcW7BkF7eTya6RPxXeJCqCJGHuCJ4GiRVLzkTXBAJMu2qaMWPrS7AANYqdq6vcBcBUdJCVVFceUvJFjaPdGZ2y9WACViL4L/0))", "sh(multi(2,[00000000/111'/222]xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y/0))", "sh(multi(2,[00000000/111h/222]xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y/0))", DEFAULT, {{"a91445a9a622a8b0a1269944be477640eedc447bbd8487"}}, OutputType::LEGACY, /*op_desc_id=*/uint256{"9339b7bfbe8cfd9d0d55819778ef77f52e5786e85b4c83be8a0d5b976e033f4c"}, {{0x8000006FUL,222},{0}}); diff --git a/src/wallet/test/ismine_tests.cpp b/src/wallet/test/ismine_tests.cpp index 3b27c7db9a0..b152f98a292 100644 --- a/src/wallet/test/ismine_tests.cpp +++ b/src/wallet/test/ismine_tests.cpp @@ -406,7 +406,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // scriptPubKey multisig - Descriptor { CWallet keystore(chain.get(), "", CreateMockableWalletDatabase()); - std::string desc_str = "multi(2, " + EncodeSecret(uncompressedKey) + ", " + EncodeSecret(keys[1]) + ")"; + std::string desc_str = "multi(2," + EncodeSecret(uncompressedKey) + "," + EncodeSecret(keys[1]) + ")"; auto spk_manager = CreateDescriptor(keystore, desc_str, true); @@ -442,7 +442,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) { CWallet keystore(chain.get(), "", CreateMockableWalletDatabase()); - std::string desc_str = "sh(multi(2, " + EncodeSecret(uncompressedKey) + ", " + EncodeSecret(keys[1]) + "))"; + std::string desc_str = "sh(multi(2," + EncodeSecret(uncompressedKey) + "," + EncodeSecret(keys[1]) + "))"; auto spk_manager = CreateDescriptor(keystore, desc_str, true); @@ -485,7 +485,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) { CWallet keystore(chain.get(), "", CreateMockableWalletDatabase()); - std::string desc_str = "wsh(multi(2, " + EncodeSecret(keys[0]) + ", " + EncodeSecret(keys[1]) + "))"; + std::string desc_str = "wsh(multi(2," + EncodeSecret(keys[0]) + "," + EncodeSecret(keys[1]) + "))"; auto spk_manager = CreateDescriptor(keystore, desc_str, true); @@ -528,7 +528,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) { CWallet keystore(chain.get(), "", CreateMockableWalletDatabase()); - std::string desc_str = "wsh(multi(2, " + EncodeSecret(uncompressedKey) + ", " + EncodeSecret(keys[1]) + "))"; + std::string desc_str = "wsh(multi(2," + EncodeSecret(uncompressedKey) + "," + EncodeSecret(keys[1]) + "))"; auto spk_manager = CreateDescriptor(keystore, desc_str, false); BOOST_CHECK_EQUAL(spk_manager, nullptr); @@ -568,7 +568,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) { CWallet keystore(chain.get(), "", CreateMockableWalletDatabase()); - std::string desc_str = "sh(wsh(multi(2, " + EncodeSecret(keys[0]) + ", " + EncodeSecret(keys[1]) + ")))"; + std::string desc_str = "sh(wsh(multi(2," + EncodeSecret(keys[0]) + "," + EncodeSecret(keys[1]) + ")))"; auto spk_manager = CreateDescriptor(keystore, desc_str, true); diff --git a/test/functional/rpc_getdescriptorinfo.py b/test/functional/rpc_getdescriptorinfo.py index e4e6d6f0f37..65aa6dd47c8 100755 --- a/test/functional/rpc_getdescriptorinfo.py +++ b/test/functional/rpc_getdescriptorinfo.py @@ -11,6 +11,7 @@ from test_framework.util import ( assert_equal, assert_raises_rpc_error, ) +from test_framework.wallet_util import generate_keypair class DescriptorTest(BitcoinTestFramework): @@ -36,6 +37,12 @@ class DescriptorTest(BitcoinTestFramework): assert_raises_rpc_error(-1, 'getdescriptorinfo', self.nodes[0].getdescriptorinfo) assert_raises_rpc_error(-3, 'JSON value of type number is not of expected type string', self.nodes[0].getdescriptorinfo, 1) assert_raises_rpc_error(-5, "'' is not a valid descriptor function", self.nodes[0].getdescriptorinfo, "") + assert_raises_rpc_error(-5, "pk(): Key ' 0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798' is invalid due to whitespace", self.nodes[0].getdescriptorinfo, "pk( 0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798)") + assert_raises_rpc_error(-5, "pk(): Key '0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798 ' is invalid due to whitespace", self.nodes[0].getdescriptorinfo, "pk(0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798 )") + assert_raises_rpc_error(-5, "Multi: Key ' 03a0434d9e47f3c86235477c7b1ae6ae5d3442d49b1943c2b752a68e2a47e247c7' is invalid due to whitespace", self.nodes[0].getdescriptorinfo, "wsh(multi(2, 03a0434d9e47f3c86235477c7b1ae6ae5d3442d49b1943c2b752a68e2a47e247c7,03774ae7f858a9411e5ef4246b70c65aac5649980be5c17891bbec17895da008cb,03d01115d548e7561b15c38f004d734633687cf4419620095bc5b0f47070afe85a))") + assert_raises_rpc_error(-5, "Multi: Key ' 03774ae7f858a9411e5ef4246b70c65aac5649980be5c17891bbec17895da008cb' is invalid due to whitespace", self.nodes[0].getdescriptorinfo, "wsh(multi(2,03a0434d9e47f3c86235477c7b1ae6ae5d3442d49b1943c2b752a68e2a47e247c7, 03774ae7f858a9411e5ef4246b70c65aac5649980be5c17891bbec17895da008cb,03d01115d548e7561b15c38f004d734633687cf4419620095bc5b0f47070afe85a))") + priv_key = generate_keypair(wif=True)[0] + assert_raises_rpc_error(-5, f"pk(): Key ' {priv_key}' is invalid due to whitespace", self.nodes[0].getdescriptorinfo, f"pk( {priv_key})") # P2PK output with the specified public key. self.test_desc('pk(0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798)', isrange=False, issolvable=True, hasprivatekeys=False) diff --git a/test/functional/wallet_importdescriptors.py b/test/functional/wallet_importdescriptors.py index f55c1a1eb31..bd50b8cdc02 100755 --- a/test/functional/wallet_importdescriptors.py +++ b/test/functional/wallet_importdescriptors.py @@ -128,6 +128,20 @@ class ImportDescriptorsTest(BitcoinTestFramework): assert_equal(info["ismine"], True) assert_equal(info["ischange"], True) + self.log.info("Should not import a descriptor with an invalid public key due to whitespace") + self.test_importdesc({"desc": descsum_create("pkh( " + key.pubkey + ")"), + "timestamp": "now", + "internal": True}, + error_code=-5, + error_message=f"pkh(): Key ' {key.pubkey}' is invalid due to whitespace", + success=False) + self.test_importdesc({"desc": descsum_create("pkh(" + key.pubkey + " )"), + "timestamp": "now", + "internal": True}, + error_code=-5, + error_message=f"pkh(): Key '{key.pubkey} ' is invalid due to whitespace", + success=False) + # # Test importing of a P2SH-P2WPKH descriptor key = get_generate_key() self.log.info("Should not import a p2sh-p2wpkh descriptor without checksum")