Merge bitcoin/bitcoin#22836: Stricter BIP32 decoding and test vector 5

56a42f10f452f0ac0e3e333646a8effcbebf6b30 Stricter BIP32 decoding and test vector 5 (Pieter Wuille)

Pull request description:

  This adds detection for various edge cases when decoding BIP32 extended pubkeys/privkeys, and tests them using the proposed https://github.com/bitcoin/bips/pull/921 BIP32 test vector 5.

ACKs for top commit:
  darosior:
    utACK 56a42f10f452f0ac0e3e333646a8effcbebf6b30 -- Had to implement essentially the same fix in python-bip32.
  kristapsk:
    ACK 56a42f10f452f0ac0e3e333646a8effcbebf6b30. Checked that test vectors are the same as in BIP32 and that tests pass.

Tree-SHA512: 5cc800cc9dc10e43ae89b659ce4f44026d04ec3cabac4eb5122d2e72ec2ed66cd5ace8c7502259e469a9ecaa5ecca2457e55dfe5fedba59948ecbf6673af67a7

3 files changed, 35 insertions, 1 deletions

diff --git a/src/key.cpp b/src/key.cpp
index 7bef3d529b..40df248e02 100644
--- a/src/key.cpp
+++ b/src/key.cpp
@@ -357,6 +357,7 @@ void CExtKey::Decode(const unsigned char code[BIP32_EXTKEY_SIZE]) {
     nChild = (code[5] << 24) | (code[6] << 16) | (code[7] << 8) | code[8];
     memcpy(chaincode.begin(), code+9, 32);
     key.Set(code+42, code+BIP32_EXTKEY_SIZE, true);
+    if ((nDepth == 0 && (nChild != 0 || ReadLE32(vchFingerprint) != 0)) || code[41] != 0) key = CKey();
 }
 
 bool ECC_InitSanityCheck() {
diff --git a/src/pubkey.cpp b/src/pubkey.cpp
index 100b315615..d14a20b870 100644
--- a/src/pubkey.cpp
+++ b/src/pubkey.cpp
@@ -350,6 +350,7 @@ void CExtPubKey::Decode(const unsigned char code[BIP32_EXTKEY_SIZE]) {
     nChild = (code[5] << 24) | (code[6] << 16) | (code[7] << 8) | code[8];
     memcpy(chaincode.begin(), code+9, 32);
     pubkey.Set(code+41, code+BIP32_EXTKEY_SIZE);
+    if ((nDepth == 0 && (nChild != 0 || ReadLE32(vchFingerprint) != 0)) || !pubkey.IsFullyValid()) pubkey = CPubKey();
 }
 
 bool CExtPubKey::Derive(CExtPubKey &out, unsigned int _nChild) const {
diff --git a/src/test/bip32_tests.cpp b/src/test/bip32_tests.cpp
index fb16c92647..a89868e1ef 100644
--- a/src/test/bip32_tests.cpp
+++ b/src/test/bip32_tests.cpp
@@ -14,6 +14,8 @@
 #include <string>
 #include <vector>
 
+namespace {
+
 struct TestDerivation {
     std::string pub;
     std::string prv;
@@ -99,7 +101,26 @@ TestVector test4 =
      "xprv9xJocDuwtYCMNAo3Zw76WENQeAS6WGXQ55RCy7tDJ8oALr4FWkuVoHJeHVAcAqiZLE7Je3vZJHxspZdFHfnBEjHqU5hG1Jaj32dVoS6XLT1",
      0);
 
-static void RunTest(const TestVector &test) {
+const std::vector<std::string> TEST5 = {
+    "xpub661MyMwAqRbcEYS8w7XLSVeEsBXy79zSzH1J8vCdxAZningWLdN3zgtU6LBpB85b3D2yc8sfvZU521AAwdZafEz7mnzBBsz4wKY5fTtTQBm",
+    "xprv9s21ZrQH143K24Mfq5zL5MhWK9hUhhGbd45hLXo2Pq2oqzMMo63oStZzFGTQQD3dC4H2D5GBj7vWvSQaaBv5cxi9gafk7NF3pnBju6dwKvH",
+    "xpub661MyMwAqRbcEYS8w7XLSVeEsBXy79zSzH1J8vCdxAZningWLdN3zgtU6Txnt3siSujt9RCVYsx4qHZGc62TG4McvMGcAUjeuwZdduYEvFn",
+    "xprv9s21ZrQH143K24Mfq5zL5MhWK9hUhhGbd45hLXo2Pq2oqzMMo63oStZzFGpWnsj83BHtEy5Zt8CcDr1UiRXuWCmTQLxEK9vbz5gPstX92JQ",
+    "xpub661MyMwAqRbcEYS8w7XLSVeEsBXy79zSzH1J8vCdxAZningWLdN3zgtU6N8ZMMXctdiCjxTNq964yKkwrkBJJwpzZS4HS2fxvyYUA4q2Xe4",
+    "xprv9s21ZrQH143K24Mfq5zL5MhWK9hUhhGbd45hLXo2Pq2oqzMMo63oStZzFAzHGBP2UuGCqWLTAPLcMtD9y5gkZ6Eq3Rjuahrv17fEQ3Qen6J",
+    "xprv9s2SPatNQ9Vc6GTbVMFPFo7jsaZySyzk7L8n2uqKXJen3KUmvQNTuLh3fhZMBoG3G4ZW1N2kZuHEPY53qmbZzCHshoQnNf4GvELZfqTUrcv",
+    "xpub661no6RGEX3uJkY4bNnPcw4URcQTrSibUZ4NqJEw5eBkv7ovTwgiT91XX27VbEXGENhYRCf7hyEbWrR3FewATdCEebj6znwMfQkhRYHRLpJ",
+    "xprv9s21ZrQH4r4TsiLvyLXqM9P7k1K3EYhA1kkD6xuquB5i39AU8KF42acDyL3qsDbU9NmZn6MsGSUYZEsuoePmjzsB3eFKSUEh3Gu1N3cqVUN",
+    "xpub661MyMwAuDcm6CRQ5N4qiHKrJ39Xe1R1NyfouMKTTWcguwVcfrZJaNvhpebzGerh7gucBvzEQWRugZDuDXjNDRmXzSZe4c7mnTK97pTvGS8",
+    "DMwo58pR1QLEFihHiXPVykYB6fJmsTeHvyTp7hRThAtCX8CvYzgPcn8XnmdfHGMQzT7ayAmfo4z3gY5KfbrZWZ6St24UVf2Qgo6oujFktLHdHY4",
+    "DMwo58pR1QLEFihHiXPVykYB6fJmsTeHvyTp7hRThAtCX8CvYzgPcn8XnmdfHPmHJiEDXkTiJTVV9rHEBUem2mwVbbNfvT2MTcAqj3nesx8uBf9",
+    "xprv9s21ZrQH143K24Mfq5zL5MhWK9hUhhGbd45hLXo2Pq2oqzMMo63oStZzF93Y5wvzdUayhgkkFoicQZcP3y52uPPxFnfoLZB21Teqt1VvEHx",
+    "xprv9s21ZrQH143K24Mfq5zL5MhWK9hUhhGbd45hLXo2Pq2oqzMMo63oStZzFAzHGBP2UuGCqWLTAPLcMtD5SDKr24z3aiUvKr9bJpdrcLg1y3G",
+    "xpub661MyMwAqRbcEYS8w7XLSVeEsBXy79zSzH1J8vCdxAZningWLdN3zgtU6Q5JXayek4PRsn35jii4veMimro1xefsM58PgBMrvdYre8QyULY",
+    "xprv9s21ZrQH143K3QTDL4LXw2F7HEK3wJUD2nW2nRk4stbPy6cq3jPPqjiChkVvvNKmPGJxWUtg6LnF5kejMRNNU3TGtRBeJgk33yuGBxrMPHL"
+};
+
+void RunTest(const TestVector &test) {
     std::vector<unsigned char> seed = ParseHex(test.strHexMaster);
     CExtKey key;
     CExtPubKey pubkey;
@@ -133,6 +154,8 @@ static void RunTest(const TestVector &test) {
     }
 }
 
+}  // namespace
+
 BOOST_FIXTURE_TEST_SUITE(bip32_tests, BasicTestingSetup)
 
 BOOST_AUTO_TEST_CASE(bip32_test1) {
@@ -151,4 +174,13 @@ BOOST_AUTO_TEST_CASE(bip32_test4) {
     RunTest(test4);
 }
 
+BOOST_AUTO_TEST_CASE(bip32_test5) {
+    for (const auto& str : TEST5) {
+        auto dec_extkey = DecodeExtKey(str);
+        auto dec_extpubkey = DecodeExtPubKey(str);
+        BOOST_CHECK_MESSAGE(!dec_extkey.key.IsValid(), "Decoding '" + str + "' as xprv should fail");
+        BOOST_CHECK_MESSAGE(!dec_extpubkey.pubkey.IsValid(), "Decoding '" + str + "' as xpub should fail");
+    }
+}
+
 BOOST_AUTO_TEST_SUITE_END()