diff --git a/src/base58.h b/src/base58.h --- a/src/base58.h +++ b/src/base58.h @@ -37,14 +37,15 @@ * return true if decoding is successful. * psz cannot be nullptr. */ -NODISCARD bool DecodeBase58(const char *psz, std::vector &vchRet); +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); + std::vector &vchRet, int max_ret_len); /** * Encode a byte vector into a base58-encoded string, including checksum @@ -55,13 +56,14 @@ * 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); +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); + std::vector &vchRet, int max_ret_len); #endif // BITCOIN_BASE58_H diff --git a/src/base58.cpp b/src/base58.cpp --- a/src/base58.cpp +++ b/src/base58.cpp @@ -11,6 +11,7 @@ #include #include #include +#include /** All alphanumeric characters except for "0", "I", "O", and "l" */ static const char *pszBase58 = @@ -32,7 +33,7 @@ -1, -1, -1, -1, -1, -1, -1, -1, -1, }; -bool DecodeBase58(const char *psz, std::vector &vch) { +bool DecodeBase58(const char *psz, std::vector &vch, int max_ret_len) { // Skip leading spaces. while (*psz && IsSpace(*psz)) { psz++; @@ -42,6 +43,9 @@ int length = 0; while (*psz == '1') { zeroes++; + if (zeroes > max_ret_len) { + return false; + } psz++; } // Allocate enough space in big-endian base256 representation. @@ -68,6 +72,9 @@ } assert(carry == 0); length = i; + if (length + zeroes > max_ret_len) { + return false; + } psz++; } // Skip trailing spaces. @@ -79,9 +86,7 @@ } // Skip leading zeroes in b256. std::vector::iterator it = b256.begin() + (size - length); - while (it != b256.end() && *it == 0) { - it++; - } + // Copy result into output vector. vch.reserve(zeroes + (b256.end() - it)); vch.assign(zeroes, 0x00); @@ -138,8 +143,9 @@ return EncodeBase58(vch.data(), vch.data() + vch.size()); } -bool DecodeBase58(const std::string &str, std::vector &vchRet) { - return DecodeBase58(str.c_str(), vchRet); +bool DecodeBase58(const std::string &str, std::vector &vchRet, + int max_ret_len) { + return DecodeBase58(str.c_str(), vchRet, max_ret_len); } std::string EncodeBase58Check(const std::vector &vchIn) { @@ -150,8 +156,13 @@ return EncodeBase58(vch); } -bool DecodeBase58Check(const char *psz, std::vector &vchRet) { - if (!DecodeBase58(psz, vchRet) || (vchRet.size() < 4)) { +bool DecodeBase58Check(const char *psz, std::vector &vchRet, + int max_ret_len) { + if (!DecodeBase58(psz, vchRet, + max_ret_len > std::numeric_limits::max() - 4 + ? std::numeric_limits::max() + : max_ret_len + 4) || + (vchRet.size() < 4)) { vchRet.clear(); return false; } @@ -165,6 +176,7 @@ return true; } -bool DecodeBase58Check(const std::string &str, std::vector &vchRet) { - return DecodeBase58Check(str.c_str(), vchRet); +bool DecodeBase58Check(const std::string &str, std::vector &vchRet, + int max_ret) { + return DecodeBase58Check(str.c_str(), vchRet, max_ret); } diff --git a/src/bench/base58.cpp b/src/bench/base58.cpp --- a/src/bench/base58.cpp +++ b/src/bench/base58.cpp @@ -33,7 +33,7 @@ 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 --- a/src/key_io.cpp +++ b/src/key_io.cpp @@ -47,7 +47,7 @@ const CChainParams ¶ms) { std::vector data; uint160 hash; - if (!DecodeBase58Check(str, data)) { + if (!DecodeBase58Check(str, data, 21)) { return CNoDestination(); } // base58-encoded Bitcoin addresses. @@ -80,7 +80,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() || @@ -113,7 +113,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() && @@ -137,7 +137,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() && diff --git a/src/test/base58_tests.cpp b/src/test/base58_tests.cpp --- a/src/test/base58_tests.cpp +++ b/src/test/base58_tests.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include @@ -56,22 +57,41 @@ } 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()); } +BOOST_AUTO_TEST_CASE(base58_random_encode_decode) { + for (int n = 0; n < 1000; ++n) { + unsigned int len = 1 + InsecureRandBits(8); + unsigned int zeroes = + InsecureRandBool() ? InsecureRandRange(len + 1) : 0; + auto data = Cat(std::vector(zeroes, '\000'), + g_insecure_rand_ctx.randbytes(len - zeroes)); + auto encoded = EncodeBase58Check(data); + std::vector decoded; + auto ok_too_small = + DecodeBase58Check(encoded, decoded, InsecureRandRange(len)); + BOOST_CHECK(!ok_too_small); + auto ok = DecodeBase58Check(encoded, decoded, + len + InsecureRandRange(257 - len)); + BOOST_CHECK(ok); + BOOST_CHECK(data == decoded); + } +} + BOOST_AUTO_TEST_SUITE_END()