Skip to content

Commit 01fa148

Browse files
committed
Merge bitcoin/bitcoin#22836: Stricter BIP32 decoding and test vector 5
56a42f1 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 bitcoin/bips#921 BIP32 test vector 5. ACKs for top commit: darosior: utACK 56a42f1 -- Had to implement essentially the same fix in python-bip32. kristapsk: ACK 56a42f1. Checked that test vectors are the same as in BIP32 and that tests pass. Tree-SHA512: 5cc800cc9dc10e43ae89b659ce4f44026d04ec3cabac4eb5122d2e72ec2ed66cd5ace8c7502259e469a9ecaa5ecca2457e55dfe5fedba59948ecbf6673af67a7
2 parents 3af495d + 56a42f1 commit 01fa148

File tree

3 files changed

+35
-1
lines changed

3 files changed

+35
-1
lines changed

src/key.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,7 @@ void CExtKey::Decode(const unsigned char code[BIP32_EXTKEY_SIZE]) {
357357
nChild = (code[5] << 24) | (code[6] << 16) | (code[7] << 8) | code[8];
358358
memcpy(chaincode.begin(), code+9, 32);
359359
key.Set(code+42, code+BIP32_EXTKEY_SIZE, true);
360+
if ((nDepth == 0 && (nChild != 0 || ReadLE32(vchFingerprint) != 0)) || code[41] != 0) key = CKey();
360361
}
361362

362363
bool ECC_InitSanityCheck() {

src/pubkey.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,7 @@ void CExtPubKey::Decode(const unsigned char code[BIP32_EXTKEY_SIZE]) {
350350
nChild = (code[5] << 24) | (code[6] << 16) | (code[7] << 8) | code[8];
351351
memcpy(chaincode.begin(), code+9, 32);
352352
pubkey.Set(code+41, code+BIP32_EXTKEY_SIZE);
353+
if ((nDepth == 0 && (nChild != 0 || ReadLE32(vchFingerprint) != 0)) || !pubkey.IsFullyValid()) pubkey = CPubKey();
353354
}
354355

355356
bool CExtPubKey::Derive(CExtPubKey &out, unsigned int _nChild) const {

src/test/bip32_tests.cpp

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
#include <string>
1515
#include <vector>
1616

17+
namespace {
18+
1719
struct TestDerivation {
1820
std::string pub;
1921
std::string prv;
@@ -99,7 +101,26 @@ TestVector test4 =
99101
"xprv9xJocDuwtYCMNAo3Zw76WENQeAS6WGXQ55RCy7tDJ8oALr4FWkuVoHJeHVAcAqiZLE7Je3vZJHxspZdFHfnBEjHqU5hG1Jaj32dVoS6XLT1",
100102
0);
101103

102-
static void RunTest(const TestVector &test) {
104+
const std::vector<std::string> TEST5 = {
105+
"xpub661MyMwAqRbcEYS8w7XLSVeEsBXy79zSzH1J8vCdxAZningWLdN3zgtU6LBpB85b3D2yc8sfvZU521AAwdZafEz7mnzBBsz4wKY5fTtTQBm",
106+
"xprv9s21ZrQH143K24Mfq5zL5MhWK9hUhhGbd45hLXo2Pq2oqzMMo63oStZzFGTQQD3dC4H2D5GBj7vWvSQaaBv5cxi9gafk7NF3pnBju6dwKvH",
107+
"xpub661MyMwAqRbcEYS8w7XLSVeEsBXy79zSzH1J8vCdxAZningWLdN3zgtU6Txnt3siSujt9RCVYsx4qHZGc62TG4McvMGcAUjeuwZdduYEvFn",
108+
"xprv9s21ZrQH143K24Mfq5zL5MhWK9hUhhGbd45hLXo2Pq2oqzMMo63oStZzFGpWnsj83BHtEy5Zt8CcDr1UiRXuWCmTQLxEK9vbz5gPstX92JQ",
109+
"xpub661MyMwAqRbcEYS8w7XLSVeEsBXy79zSzH1J8vCdxAZningWLdN3zgtU6N8ZMMXctdiCjxTNq964yKkwrkBJJwpzZS4HS2fxvyYUA4q2Xe4",
110+
"xprv9s21ZrQH143K24Mfq5zL5MhWK9hUhhGbd45hLXo2Pq2oqzMMo63oStZzFAzHGBP2UuGCqWLTAPLcMtD9y5gkZ6Eq3Rjuahrv17fEQ3Qen6J",
111+
"xprv9s2SPatNQ9Vc6GTbVMFPFo7jsaZySyzk7L8n2uqKXJen3KUmvQNTuLh3fhZMBoG3G4ZW1N2kZuHEPY53qmbZzCHshoQnNf4GvELZfqTUrcv",
112+
"xpub661no6RGEX3uJkY4bNnPcw4URcQTrSibUZ4NqJEw5eBkv7ovTwgiT91XX27VbEXGENhYRCf7hyEbWrR3FewATdCEebj6znwMfQkhRYHRLpJ",
113+
"xprv9s21ZrQH4r4TsiLvyLXqM9P7k1K3EYhA1kkD6xuquB5i39AU8KF42acDyL3qsDbU9NmZn6MsGSUYZEsuoePmjzsB3eFKSUEh3Gu1N3cqVUN",
114+
"xpub661MyMwAuDcm6CRQ5N4qiHKrJ39Xe1R1NyfouMKTTWcguwVcfrZJaNvhpebzGerh7gucBvzEQWRugZDuDXjNDRmXzSZe4c7mnTK97pTvGS8",
115+
"DMwo58pR1QLEFihHiXPVykYB6fJmsTeHvyTp7hRThAtCX8CvYzgPcn8XnmdfHGMQzT7ayAmfo4z3gY5KfbrZWZ6St24UVf2Qgo6oujFktLHdHY4",
116+
"DMwo58pR1QLEFihHiXPVykYB6fJmsTeHvyTp7hRThAtCX8CvYzgPcn8XnmdfHPmHJiEDXkTiJTVV9rHEBUem2mwVbbNfvT2MTcAqj3nesx8uBf9",
117+
"xprv9s21ZrQH143K24Mfq5zL5MhWK9hUhhGbd45hLXo2Pq2oqzMMo63oStZzF93Y5wvzdUayhgkkFoicQZcP3y52uPPxFnfoLZB21Teqt1VvEHx",
118+
"xprv9s21ZrQH143K24Mfq5zL5MhWK9hUhhGbd45hLXo2Pq2oqzMMo63oStZzFAzHGBP2UuGCqWLTAPLcMtD5SDKr24z3aiUvKr9bJpdrcLg1y3G",
119+
"xpub661MyMwAqRbcEYS8w7XLSVeEsBXy79zSzH1J8vCdxAZningWLdN3zgtU6Q5JXayek4PRsn35jii4veMimro1xefsM58PgBMrvdYre8QyULY",
120+
"xprv9s21ZrQH143K3QTDL4LXw2F7HEK3wJUD2nW2nRk4stbPy6cq3jPPqjiChkVvvNKmPGJxWUtg6LnF5kejMRNNU3TGtRBeJgk33yuGBxrMPHL"
121+
};
122+
123+
void RunTest(const TestVector &test) {
103124
std::vector<unsigned char> seed = ParseHex(test.strHexMaster);
104125
CExtKey key;
105126
CExtPubKey pubkey;
@@ -133,6 +154,8 @@ static void RunTest(const TestVector &test) {
133154
}
134155
}
135156

157+
} // namespace
158+
136159
BOOST_FIXTURE_TEST_SUITE(bip32_tests, BasicTestingSetup)
137160

138161
BOOST_AUTO_TEST_CASE(bip32_test1) {
@@ -151,4 +174,13 @@ BOOST_AUTO_TEST_CASE(bip32_test4) {
151174
RunTest(test4);
152175
}
153176

177+
BOOST_AUTO_TEST_CASE(bip32_test5) {
178+
for (const auto& str : TEST5) {
179+
auto dec_extkey = DecodeExtKey(str);
180+
auto dec_extpubkey = DecodeExtPubKey(str);
181+
BOOST_CHECK_MESSAGE(!dec_extkey.key.IsValid(), "Decoding '" + str + "' as xprv should fail");
182+
BOOST_CHECK_MESSAGE(!dec_extpubkey.pubkey.IsValid(), "Decoding '" + str + "' as xpub should fail");
183+
}
184+
}
185+
154186
BOOST_AUTO_TEST_SUITE_END()

0 commit comments

Comments
 (0)