From 5909bcd3bf3c3502355e89fd0b76bb8e93d8a95b Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 18 Nov 2019 15:26:55 -0800 Subject: [PATCH] Add bounds checks in key_io before DecodeBase58Check --- src/base58.h | 9 ++++----- src/bench/base58.cpp | 2 +- src/key_io.cpp | 8 ++++---- src/test/base58_tests.cpp | 8 ++++---- 4 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/base58.h b/src/base58.h index cfdab511b6..90eded4992 100644 --- a/src/base58.h +++ b/src/base58.h @@ -16,7 +16,6 @@ #include -#include #include #include @@ -36,13 +35,13 @@ std::string EncodeBase58(const std::vector& vch); * return true if decoding is successful. * psz cannot be nullptr. */ -NODISCARD bool DecodeBase58(const char* psz, std::vector& vchRet, int max_ret_len = std::numeric_limits::max()); +NODISCARD bool DecodeBase58(const char* psz, std::vector& vchRet, int max_ret_len); /** * Decode a base58-encoded string (str) into a byte vector (vchRet). * return true if decoding is successful. */ -NODISCARD bool DecodeBase58(const std::string& str, std::vector& vchRet, int max_ret_len = std::numeric_limits::max()); +NODISCARD bool DecodeBase58(const std::string& str, std::vector& vchRet, int max_ret_len); /** * Encode a byte vector into a base58-encoded string, including checksum @@ -53,12 +52,12 @@ std::string EncodeBase58Check(const std::vector& vchIn); * Decode a base58-encoded string (psz) that includes a checksum into a byte * vector (vchRet), return true if decoding is successful */ -NODISCARD bool DecodeBase58Check(const char* psz, std::vector& vchRet, int max_ret_len = std::numeric_limits::max()); +NODISCARD bool DecodeBase58Check(const char* psz, std::vector& vchRet, int max_ret_len); /** * Decode a base58-encoded string (str) that includes a checksum into a byte * vector (vchRet), return true if decoding is successful */ -NODISCARD bool DecodeBase58Check(const std::string& str, std::vector& vchRet, int max_ret_len = std::numeric_limits::max()); +NODISCARD bool DecodeBase58Check(const std::string& str, std::vector& vchRet, int max_ret_len); #endif // BITCOIN_BASE58_H diff --git a/src/bench/base58.cpp b/src/bench/base58.cpp index 40a7b5e320..d8c9b2e01a 100644 --- a/src/bench/base58.cpp +++ b/src/bench/base58.cpp @@ -47,7 +47,7 @@ static void Base58Decode(benchmark::State& state) const char* addr = "17VZNX1SN5NtKa8UQFxwQbFeFc3iqRYhem"; std::vector vch; while (state.KeepRunning()) { - (void) DecodeBase58(addr, vch); + (void) DecodeBase58(addr, vch, 64); } } diff --git a/src/key_io.cpp b/src/key_io.cpp index 363055d6b3..af06db7343 100644 --- a/src/key_io.cpp +++ b/src/key_io.cpp @@ -73,7 +73,7 @@ CTxDestination DecodeDestination(const std::string& str, const CChainParams& par { std::vector data; uint160 hash; - if (DecodeBase58Check(str, data)) { + if (DecodeBase58Check(str, data, 21)) { // base58-encoded Bitcoin addresses. // Public-key-hash-addresses have version 0 (or 111 testnet). // The data vector contains RIPEMD160(SHA256(pubkey)), where pubkey is the serialized public key. @@ -133,7 +133,7 @@ CKey DecodeSecret(const std::string& str) { CKey key; std::vector data; - if (DecodeBase58Check(str, data)) { + if (DecodeBase58Check(str, data, 34)) { const std::vector& privkey_prefix = Params().Base58Prefix(CChainParams::SECRET_KEY); if ((data.size() == 32 + privkey_prefix.size() || (data.size() == 33 + privkey_prefix.size() && data.back() == 1)) && std::equal(privkey_prefix.begin(), privkey_prefix.end(), data.begin())) { @@ -164,7 +164,7 @@ CExtPubKey DecodeExtPubKey(const std::string& str) { CExtPubKey key; std::vector data; - if (DecodeBase58Check(str, data)) { + if (DecodeBase58Check(str, data, 78)) { const std::vector& prefix = Params().Base58Prefix(CChainParams::EXT_PUBLIC_KEY); if (data.size() == BIP32_EXTKEY_SIZE + prefix.size() && std::equal(prefix.begin(), prefix.end(), data.begin())) { key.Decode(data.data() + prefix.size()); @@ -187,7 +187,7 @@ CExtKey DecodeExtKey(const std::string& str) { CExtKey key; std::vector data; - if (DecodeBase58Check(str, data)) { + if (DecodeBase58Check(str, data, 78)) { const std::vector& prefix = Params().Base58Prefix(CChainParams::EXT_SECRET_KEY); if (data.size() == BIP32_EXTKEY_SIZE + prefix.size() && std::equal(prefix.begin(), prefix.end(), data.begin())) { key.Decode(data.data() + prefix.size()); diff --git a/src/test/base58_tests.cpp b/src/test/base58_tests.cpp index 5d53088cc6..96fdf8c86d 100644 --- a/src/test/base58_tests.cpp +++ b/src/test/base58_tests.cpp @@ -54,15 +54,15 @@ BOOST_AUTO_TEST_CASE(base58_DecodeBase58) } std::vector expected = ParseHex(test[0].get_str()); std::string base58string = test[1].get_str(); - BOOST_CHECK_MESSAGE(DecodeBase58(base58string, result), strTest); + BOOST_CHECK_MESSAGE(DecodeBase58(base58string, result, 256), strTest); BOOST_CHECK_MESSAGE(result.size() == expected.size() && std::equal(result.begin(), result.end(), expected.begin()), strTest); } - BOOST_CHECK(!DecodeBase58("invalid", result)); + BOOST_CHECK(!DecodeBase58("invalid", result, 100)); // check that DecodeBase58 skips whitespace, but still fails with unexpected non-whitespace at the end. - BOOST_CHECK(!DecodeBase58(" \t\n\v\f\r skip \r\f\v\n\t a", result)); - BOOST_CHECK( DecodeBase58(" \t\n\v\f\r skip \r\f\v\n\t ", result)); + BOOST_CHECK(!DecodeBase58(" \t\n\v\f\r skip \r\f\v\n\t a", result, 3)); + BOOST_CHECK( DecodeBase58(" \t\n\v\f\r skip \r\f\v\n\t ", result, 3)); std::vector expected = ParseHex("971a55"); BOOST_CHECK_EQUAL_COLLECTIONS(result.begin(), result.end(), expected.begin(), expected.end()); }