Merge #13712: wallet: Fix non-determinism in ParseHDKeypath(...). Avoid using an uninitialized variable in path calculation.

27ee53c1ae wallet: Add error handling. Check return value of ParseUInt32(...) in ParseHDKeypath(...). (practicalswift)
7223263899 wallet: Add tests for ParseHDKeypath(...) (practicalswift)

Pull request description:

  Add error handling. Check return value of `ParseUInt32(...)` in `ParseHDKeypath(...)`.

  `ParseUInt32(...)` returns `false` if the entire string could not be parsed or when an overflow or underflow occurred. In such case the uninitialized variable `number` would be used in the calculation of `path` (prior to this commit).

  An example key path triggering this is `m/0/4294967296`:

  ```
    ParseHDKeypath("m/0/4294967296", keypath);
  ```

  `4294967296` is `1` + `0xFFFFFFFF` (`uint32_t` max: `4294967295`).

  Introduced in a4b06fb42e which was merged into `master` 14 hours ago as part of #13557 ("BIP 174 PSBT Serializations and RPCs").

Tree-SHA512: e5ff423f67c18d82c1231bde6343587a453e793c32004d93dc9b61be6d9372b57a6b2c9978d9eb1000d6cc82fd180f2486013f928dca737fb92daad22c16e467
This commit is contained in:
MarcoFalke 2018-07-19 17:39:48 -04:00
commit aba2e666d7
No known key found for this signature in database
GPG key ID: D2EA4850E7528B25
2 changed files with 79 additions and 1 deletions

View file

@ -4433,7 +4433,9 @@ bool ParseHDKeypath(std::string keypath_str, std::vector<uint32_t>& keypath)
return false; return false;
} }
uint32_t number; uint32_t number;
ParseUInt32(item, &number); if (!ParseUInt32(item, &number)) {
return false;
}
path |= number; path |= number;
keypath.push_back(path); keypath.push_back(path);

View file

@ -13,6 +13,8 @@
#include <test/test_bitcoin.h> #include <test/test_bitcoin.h>
#include <wallet/test/wallet_test_fixture.h> #include <wallet/test/wallet_test_fixture.h>
extern bool ParseHDKeypath(std::string keypath_str, std::vector<uint32_t>& keypath);
BOOST_FIXTURE_TEST_SUITE(psbt_wallet_tests, WalletTestingSetup) BOOST_FIXTURE_TEST_SUITE(psbt_wallet_tests, WalletTestingSetup)
BOOST_AUTO_TEST_CASE(psbt_updater_test) BOOST_AUTO_TEST_CASE(psbt_updater_test)
@ -71,4 +73,78 @@ BOOST_AUTO_TEST_CASE(psbt_updater_test)
BOOST_CHECK_EQUAL(final_hex, "70736274ff01009a020000000258e87a21b56daf0c23be8e7070456c336f7cbaa5c8757924f545887bb2abdd750000000000ffffffff838d0427d0ec650a68aa46bb0b098aea4422c071b2ca78352a077959d07cea1d0100000000ffffffff0270aaf00800000000160014d85c2b71d0060b09c9886aeb815e50991dda124d00e1f5050000000016001400aea9a2e5f0f876a588df5546e8742d1d87008f00000000000100bb0200000001aad73931018bd25f84ae400b68848be09db706eac2ac18298babee71ab656f8b0000000048473044022058f6fc7c6a33e1b31548d481c826c015bd30135aad42cd67790dab66d2ad243b02204a1ced2604c6735b6393e5b41691dd78b00f0c5942fb9f751856faa938157dba01feffffff0280f0fa020000000017a9140fb9463421696b82c833af241c78c17ddbde493487d0f20a270100000017a91429ca74f8a08f81999428185c97b5d852e4063f6187650000000104475221029583bf39ae0a609747ad199addd634fa6108559d6c5cd39b4c2183f1ab96e07f2102dab61ff49a14db6a7d02b0cd1fbb78fc4b18312b5b4e54dae4dba2fbfef536d752ae2206029583bf39ae0a609747ad199addd634fa6108559d6c5cd39b4c2183f1ab96e07f10d90c6a4f000000800000008000000080220602dab61ff49a14db6a7d02b0cd1fbb78fc4b18312b5b4e54dae4dba2fbfef536d710d90c6a4f0000008000000080010000800001012000c2eb0b0000000017a914b7f5faf40e3d40a5a459b1db3535f2b72fa921e88701042200208c2353173743b595dfb4a07b72ba8e42e3797da74e87fe7d9d7497e3b2028903010547522103089dc10c7ac6db54f91329af617333db388cead0c231f723379d1b99030b02dc21023add904f3d6dcf59ddb906b0dee23529b7ffb9ed50e5e86151926860221f0e7352ae2206023add904f3d6dcf59ddb906b0dee23529b7ffb9ed50e5e86151926860221f0e7310d90c6a4f000000800000008003000080220603089dc10c7ac6db54f91329af617333db388cead0c231f723379d1b99030b02dc10d90c6a4f00000080000000800200008000220203a9a4c37f5996d3aa25dbac6b570af0650394492942460b354753ed9eeca5877110d90c6a4f000000800000008004000080002202027f6399757d2eff55a136ad02c684b1838b6556e5f1b6b34282a94b6b5005109610d90c6a4f00000080000000800500008000"); BOOST_CHECK_EQUAL(final_hex, "70736274ff01009a020000000258e87a21b56daf0c23be8e7070456c336f7cbaa5c8757924f545887bb2abdd750000000000ffffffff838d0427d0ec650a68aa46bb0b098aea4422c071b2ca78352a077959d07cea1d0100000000ffffffff0270aaf00800000000160014d85c2b71d0060b09c9886aeb815e50991dda124d00e1f5050000000016001400aea9a2e5f0f876a588df5546e8742d1d87008f00000000000100bb0200000001aad73931018bd25f84ae400b68848be09db706eac2ac18298babee71ab656f8b0000000048473044022058f6fc7c6a33e1b31548d481c826c015bd30135aad42cd67790dab66d2ad243b02204a1ced2604c6735b6393e5b41691dd78b00f0c5942fb9f751856faa938157dba01feffffff0280f0fa020000000017a9140fb9463421696b82c833af241c78c17ddbde493487d0f20a270100000017a91429ca74f8a08f81999428185c97b5d852e4063f6187650000000104475221029583bf39ae0a609747ad199addd634fa6108559d6c5cd39b4c2183f1ab96e07f2102dab61ff49a14db6a7d02b0cd1fbb78fc4b18312b5b4e54dae4dba2fbfef536d752ae2206029583bf39ae0a609747ad199addd634fa6108559d6c5cd39b4c2183f1ab96e07f10d90c6a4f000000800000008000000080220602dab61ff49a14db6a7d02b0cd1fbb78fc4b18312b5b4e54dae4dba2fbfef536d710d90c6a4f0000008000000080010000800001012000c2eb0b0000000017a914b7f5faf40e3d40a5a459b1db3535f2b72fa921e88701042200208c2353173743b595dfb4a07b72ba8e42e3797da74e87fe7d9d7497e3b2028903010547522103089dc10c7ac6db54f91329af617333db388cead0c231f723379d1b99030b02dc21023add904f3d6dcf59ddb906b0dee23529b7ffb9ed50e5e86151926860221f0e7352ae2206023add904f3d6dcf59ddb906b0dee23529b7ffb9ed50e5e86151926860221f0e7310d90c6a4f000000800000008003000080220603089dc10c7ac6db54f91329af617333db388cead0c231f723379d1b99030b02dc10d90c6a4f00000080000000800200008000220203a9a4c37f5996d3aa25dbac6b570af0650394492942460b354753ed9eeca5877110d90c6a4f000000800000008004000080002202027f6399757d2eff55a136ad02c684b1838b6556e5f1b6b34282a94b6b5005109610d90c6a4f00000080000000800500008000");
} }
BOOST_AUTO_TEST_CASE(parse_hd_keypath)
{
std::vector<uint32_t> keypath;
BOOST_CHECK(ParseHDKeypath("1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1", keypath));
BOOST_CHECK(!ParseHDKeypath("///////////////////////////", keypath));
BOOST_CHECK(ParseHDKeypath("1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1'/1", keypath));
BOOST_CHECK(!ParseHDKeypath("//////////////////////////'/", keypath));
BOOST_CHECK(ParseHDKeypath("1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/", keypath));
BOOST_CHECK(!ParseHDKeypath("1///////////////////////////", keypath));
BOOST_CHECK(ParseHDKeypath("1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1/1'/", keypath));
BOOST_CHECK(!ParseHDKeypath("1/'//////////////////////////", keypath));
BOOST_CHECK(ParseHDKeypath("", keypath));
BOOST_CHECK(!ParseHDKeypath(" ", keypath));
BOOST_CHECK(ParseHDKeypath("0", keypath));
BOOST_CHECK(!ParseHDKeypath("O", keypath));
BOOST_CHECK(ParseHDKeypath("0000'/0000'/0000'", keypath));
BOOST_CHECK(!ParseHDKeypath("0000,/0000,/0000,", keypath));
BOOST_CHECK(ParseHDKeypath("01234", keypath));
BOOST_CHECK(!ParseHDKeypath("0x1234", keypath));
BOOST_CHECK(ParseHDKeypath("1", keypath));
BOOST_CHECK(!ParseHDKeypath(" 1", keypath));
BOOST_CHECK(ParseHDKeypath("42", keypath));
BOOST_CHECK(!ParseHDKeypath("m42", keypath));
BOOST_CHECK(ParseHDKeypath("4294967295", keypath)); // 4294967295 == 0xFFFFFFFF (uint32_t max)
BOOST_CHECK(!ParseHDKeypath("4294967296", keypath)); // 4294967296 == 0xFFFFFFFF (uint32_t max) + 1
BOOST_CHECK(ParseHDKeypath("m", keypath));
BOOST_CHECK(!ParseHDKeypath("n", keypath));
BOOST_CHECK(ParseHDKeypath("m/", keypath));
BOOST_CHECK(!ParseHDKeypath("n/", keypath));
BOOST_CHECK(ParseHDKeypath("m/0", keypath));
BOOST_CHECK(!ParseHDKeypath("n/0", keypath));
BOOST_CHECK(ParseHDKeypath("m/0'", keypath));
BOOST_CHECK(!ParseHDKeypath("m/0''", keypath));
BOOST_CHECK(ParseHDKeypath("m/0'/0'", keypath));
BOOST_CHECK(!ParseHDKeypath("m/'0/0'", keypath));
BOOST_CHECK(ParseHDKeypath("m/0/0", keypath));
BOOST_CHECK(!ParseHDKeypath("n/0/0", keypath));
BOOST_CHECK(ParseHDKeypath("m/0/0/00", keypath));
BOOST_CHECK(!ParseHDKeypath("m/0/0/f00", keypath));
BOOST_CHECK(ParseHDKeypath("m/0/0/000000000000000000000000000000000000000000000000000000000000000000000000000000000000", keypath));
BOOST_CHECK(!ParseHDKeypath("m/1/1/111111111111111111111111111111111111111111111111111111111111111111111111111111111111", keypath));
BOOST_CHECK(ParseHDKeypath("m/0/00/0", keypath));
BOOST_CHECK(!ParseHDKeypath("m/0'/00/'0", keypath));
BOOST_CHECK(ParseHDKeypath("m/1/", keypath));
BOOST_CHECK(!ParseHDKeypath("m/1//", keypath));
BOOST_CHECK(ParseHDKeypath("m/0/4294967295", keypath)); // 4294967295 == 0xFFFFFFFF (uint32_t max)
BOOST_CHECK(!ParseHDKeypath("m/0/4294967296", keypath)); // 4294967296 == 0xFFFFFFFF (uint32_t max) + 1
BOOST_CHECK(ParseHDKeypath("m/4294967295", keypath)); // 4294967295 == 0xFFFFFFFF (uint32_t max)
BOOST_CHECK(!ParseHDKeypath("m/4294967296", keypath)); // 4294967296 == 0xFFFFFFFF (uint32_t max) + 1
}
BOOST_AUTO_TEST_SUITE_END() BOOST_AUTO_TEST_SUITE_END()