Pass a maximum output length to DecodeBase58 and DecodeBase58Check

Also remove a needless loop in DecodeBase58 to prune zeroes in the base256
output of the conversion. The number of zeroes is implied by keeping track
explicitly of the length during the loop.

Author: sipa

src/base58.cpp

@@ -11,6 +11,8 @@
 #include <assert.h>
 #include <string.h>
 
+#include <limits>
+
 /** All alphanumeric characters except for "0", "I", "O", and "l" */
 static const char* pszBase58 = "123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz";
 static const int8_t mapBase58[256] = {
@@ -32,7 +34,7 @@ static const int8_t mapBase58[256] = {
     -1,-1,-1,-1,-1,-1,-1,-1, -1,-1,-1,-1,-1,-1,-1,-1,
 };
 
-bool DecodeBase58(const char* psz, std::vector<unsigned char>& vch)
+bool DecodeBase58(const char* psz, std::vector<unsigned char>& vch, int max_ret_len)
 {
     // Skip leading spaces.
     while (*psz && IsSpace(*psz))
@@ -42,6 +44,7 @@ bool DecodeBase58(const char* psz, std::vector<unsigned char>& vch)
     int length = 0;
     while (*psz == '1') {
         zeroes++;
+        if (zeroes > max_ret_len) return false;
         psz++;
     }
     // Allocate enough space in big-endian base256 representation.
@@ -62,6 +65,7 @@ bool DecodeBase58(const char* psz, std::vector<unsigned char>& vch)
         }
         assert(carry == 0);
         length = i;
+        if (length + zeroes > max_ret_len) return false;
         psz++;
     }
     // Skip trailing spaces.
@@ -71,8 +75,6 @@ bool DecodeBase58(const char* psz, std::vector<unsigned char>& vch)
         return false;
     // Skip leading zeroes in b256.
     std::vector<unsigned char>::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);
@@ -126,9 +128,9 @@ std::string EncodeBase58(const std::vector<unsigned char>& vch)
     return EncodeBase58(vch.data(), vch.data() + vch.size());
 }
 
-bool DecodeBase58(const std::string& str, std::vector<unsigned char>& vchRet)
+bool DecodeBase58(const std::string& str, std::vector<unsigned char>& vchRet, int max_ret_len)
 {
-    return DecodeBase58(str.c_str(), vchRet);
+    return DecodeBase58(str.c_str(), vchRet, max_ret_len);
 }
 
 std::string EncodeBase58Check(const std::vector<unsigned char>& vchIn)
@@ -140,9 +142,9 @@ std::string EncodeBase58Check(const std::vector<unsigned char>& vchIn)
     return EncodeBase58(vch);
 }
 
-bool DecodeBase58Check(const char* psz, std::vector<unsigned char>& vchRet)
+bool DecodeBase58Check(const char* psz, std::vector<unsigned char>& vchRet, int max_ret_len)
 {
-    if (!DecodeBase58(psz, vchRet) ||
+    if (!DecodeBase58(psz, vchRet, max_ret_len > std::numeric_limits<int>::max() - 4 ? std::numeric_limits<int>::max() : max_ret_len + 4) ||
         (vchRet.size() < 4)) {
         vchRet.clear();
         return false;
@@ -157,7 +159,7 @@ bool DecodeBase58Check(const char* psz, std::vector<unsigned char>& vchRet)
     return true;
 }
 
-bool DecodeBase58Check(const std::string& str, std::vector<unsigned char>& vchRet)
+bool DecodeBase58Check(const std::string& str, std::vector<unsigned char>& vchRet, int max_ret)
 {
-    return DecodeBase58Check(str.c_str(), vchRet);
+    return DecodeBase58Check(str.c_str(), vchRet, max_ret);
 }

src/base58.h

@@ -16,6 +16,7 @@
 
 #include <attributes.h>
 
+#include <limits>
 #include <string>
 #include <vector>
 
@@ -35,13 +36,13 @@ std::string EncodeBase58(const std::vector<unsigned char>& vch);
  * return true if decoding is successful.
  * psz cannot be nullptr.
  */
-NODISCARD bool DecodeBase58(const char* psz, std::vector<unsigned char>& vchRet);
+NODISCARD bool DecodeBase58(const char* psz, std::vector<unsigned char>& vchRet, int max_ret_len = std::numeric_limits<int>::max());
 
 /**
  * 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<unsigned char>& vchRet);
+NODISCARD bool DecodeBase58(const std::string& str, std::vector<unsigned char>& vchRet, int max_ret_len = std::numeric_limits<int>::max());
 
 /**
  * Encode a byte vector into a base58-encoded string, including checksum
@@ -52,12 +53,12 @@ std::string EncodeBase58Check(const std::vector<unsigned char>& 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<unsigned char>& vchRet);
+NODISCARD bool DecodeBase58Check(const char* psz, std::vector<unsigned char>& vchRet, int max_ret_len = std::numeric_limits<int>::max());
 
 /**
  * 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<unsigned char>& vchRet);
+NODISCARD bool DecodeBase58Check(const std::string& str, std::vector<unsigned char>& vchRet, int max_ret_len = std::numeric_limits<int>::max());
 
 #endif // BITCOIN_BASE58_H

src/test/base58_tests.cpp

@@ -7,6 +7,7 @@
 #include <base58.h>
 #include <test/util/setup_common.h>
 #include <util/strencodings.h>
+#include <util/vector.h>
 
 #include <univalue.h>
 
@@ -66,4 +67,20 @@ BOOST_AUTO_TEST_CASE(base58_DecodeBase58)
     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<unsigned char>(zeroes, '\000'), g_insecure_rand_ctx.randbytes(len - zeroes));
+        auto encoded = EncodeBase58Check(data);
+        std::vector<unsigned char> 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()