From 50856695ef6c02ecbaa0cf448567355b6b86b510 Mon Sep 17 00:00:00 2001 From: brunoerg Date: Fri, 10 Jan 2025 18:13:27 -0300 Subject: [PATCH 1/5] test: fix descriptors in `ismine_tests` Some descriptors contain whitespace in public keys within fragments. This fixes it. --- src/wallet/test/ismine_tests.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) 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); From cb722a3cea16a04844c83e56fd6deaa1f0dc0a7e Mon Sep 17 00:00:00 2001 From: brunoerg Date: Tue, 31 Dec 2024 11:16:42 -0300 Subject: [PATCH 2/5] descriptor: check whitespace in ParsePubkeyInner Due to Base58, keys with whitespace at the beginning or at the end are successfully parsed. This commit adds a check into `ParsePubkeyInner` to verify whether if the first or last char of the key is a space. --- src/script/descriptor.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index 499af47ee55..79959dbf4c5 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); From c7afca3d62cf5d3ea9b98d5a76e4e54cac07bc3c Mon Sep 17 00:00:00 2001 From: brunoerg Date: Fri, 3 Jan 2025 14:20:34 -0300 Subject: [PATCH 3/5] test: descriptor: check whitespace into keys --- src/test/descriptor_tests.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/test/descriptor_tests.cpp b/src/test/descriptor_tests.cpp index 1b4020e0a06..8207cc53c79 100644 --- a/src/test/descriptor_tests.cpp +++ b/src/test/descriptor_tests.cpp @@ -899,6 +899,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}}); From a8b548d75d9a376c9bb66e06bb918c876416d615 Mon Sep 17 00:00:00 2001 From: brunoerg Date: Wed, 8 Jan 2025 12:26:44 -0300 Subject: [PATCH 4/5] test: `getdescriptorinfo`/`importdescriptors` with whitespace in pubkeys --- test/functional/rpc_getdescriptorinfo.py | 7 +++++++ test/functional/wallet_importdescriptors.py | 14 ++++++++++++++ 2 files changed, 21 insertions(+) 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 84c07b6a282..5aabd5db331 100755 --- a/test/functional/wallet_importdescriptors.py +++ b/test/functional/wallet_importdescriptors.py @@ -129,6 +129,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") From 21e9d39a3725cd6107b742f0cb97f65b3640201b Mon Sep 17 00:00:00 2001 From: brunoerg Date: Wed, 8 Jan 2025 17:41:47 -0300 Subject: [PATCH 5/5] docs: add release notes for 31603 --- doc/release-notes-31603.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 doc/release-notes-31603.md 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 )`.