Use error codes throughout fast Base58 implementation

thejohnfreeman · ximinez · commit c157816017ae · 2024-07-24T19:37:34.000-04:00
diff --git a/src/ripple/protocol/impl/b58_utils.h b/src/ripple/protocol/impl/b58_utils.h
@@ -21,6 +21,7 @@
 #define RIPPLE_PROTOCOL_B58_UTILS_H_INCLUDED
 
 #include <ripple/basics/contract.h>
+#include <ripple/protocol/impl/token_errors.h>
 
 #include <boost/outcome.hpp>
 #include <boost/outcome/result.hpp>
@@ -71,12 +72,12 @@ carrying_add(std::uint64_t a, std::uint64_t b)
 // (i.e a[0] is the 2^0 coefficient, a[n] is the 2^(64*n) coefficient)
 // panics if overflows (this is a specialized adder for b58 decoding.
 // it should never overflow).
-inline void
+[[nodiscard]] inline TokenCodecErrc
 inplace_bigint_add(std::span<std::uint64_t> a, std::uint64_t b)
 {
     if (a.size() <= 1)
     {
-        ripple::LogicError("Input span too small for inplace_bigint_add");
+        return TokenCodecErrc::inputTooSmall;
     }
 
     std::uint64_t carry;
@@ -86,28 +87,29 @@ inplace_bigint_add(std::span<std::uint64_t> a, std::uint64_t b)
     {
         if (!carry)
         {
-            return;
+            return TokenCodecErrc::success;
         }
         std::tie(v, carry) = carrying_add(v, 1);
     }
     if (carry)
     {
-        LogicError("Overflow in inplace_bigint_add");
+        return TokenCodecErrc::overflowAdd;
     }
+    return TokenCodecErrc::success;
 }
 
-inline void
+[[nodiscard]] inline TokenCodecErrc
 inplace_bigint_mul(std::span<std::uint64_t> a, std::uint64_t b)
 {
     if (a.empty())
     {
-        LogicError("Empty span passed to inplace_bigint_mul");
+        return TokenCodecErrc::inputTooSmall;
     }
 
     auto const last_index = a.size() - 1;
     if (a[last_index] != 0)
     {
-        LogicError("Non-zero element in inplace_bigint_mul last index");
+        return TokenCodecErrc::inputTooLarge;
     }
 
     std::uint64_t carry = 0;
@@ -116,7 +118,9 @@ inplace_bigint_mul(std::span<std::uint64_t> a, std::uint64_t b)
         std::tie(coeff, carry) = carrying_mul(coeff, b, carry);
     }
     a[last_index] = carry;
+    return TokenCodecErrc::success;
 }
+
 // divide a "big uint" value inplace and return the mod
 // numerator is stored so smallest coefficients come first
 [[nodiscard]] inline std::uint64_t
@@ -166,11 +170,7 @@ inplace_bigint_div_rem(std::span<uint64_t> numerator, std::uint64_t divisor)
 b58_10_to_b58_be(std::uint64_t input)
 {
     constexpr std::uint64_t B_58_10 = 430804206899405824;  // 58^10;
-    if (input >= B_58_10)
-    {
-        LogicError("Input to b58_10_to_b58_be equals or exceeds 58^10.");
-    }
-
+    assert(input < B_58_10);
     constexpr std::size_t resultSize = 10;
     std::array<std::uint8_t, resultSize> result{};
     int i = 0;
diff --git a/src/ripple/protocol/impl/token_errors.h b/src/ripple/protocol/impl/token_errors.h
@@ -32,6 +32,7 @@ enum class TokenCodecErrc {
     mismatchedTokenType,
     mismatchedChecksum,
     invalidEncodingChar,
+    overflowAdd,
     unknown,
 };
 }
diff --git a/src/ripple/protocol/impl/tokens.cpp b/src/ripple/protocol/impl/tokens.cpp
@@ -467,6 +467,11 @@ b256_to_b58_be(std::span<std::uint8_t const> input, std::span<std::uint8_t> out)
         {
             continue;
         }
+        static constexpr std::uint64_t B_58_10 = 430804206899405824;  // 58^10;
+        if (base_58_10_coeff[i] >= B_58_10)
+        {
+            return Unexpected(TokenCodecErrc::inputTooLarge);
+        }
         std::array<std::uint8_t, 10> const b58_be =
             ripple::b58_fast::detail::b58_10_to_b58_be(base_58_10_coeff[i]);
         std::size_t to_skip = 0;
@@ -565,10 +570,23 @@ b58_to_b256_be(std::string_view input, std::span<std::uint8_t> out)
     for (int i = 1; i < num_b_58_10_coeffs; ++i)
     {
         std::uint64_t const c = b_58_10_coeff[i];
-        ripple::b58_fast::detail::inplace_bigint_mul(
-            std::span(&result[0], cur_result_size + 1), B_58_10);
-        ripple::b58_fast::detail::inplace_bigint_add(
-            std::span(&result[0], cur_result_size + 1), c);
+
+        {
+            auto code = ripple::b58_fast::detail::inplace_bigint_mul(
+                std::span(&result[0], cur_result_size + 1), B_58_10);
+            if (code != TokenCodecErrc::success)
+            {
+                return Unexpected(code);
+            }
+        }
+        {
+            auto code = ripple::b58_fast::detail::inplace_bigint_add(
+                std::span(&result[0], cur_result_size + 1), c);
+            if (code != TokenCodecErrc::success)
+            {
+                return Unexpected(code);
+            }
+        }
         if (result[cur_result_size] != 0)
         {
             cur_result_size += 1;
diff --git a/src/test/basics/base58_test.cpp b/src/test/basics/base58_test.cpp
@@ -177,6 +177,7 @@ class base58_test : public beast::unit_test::suite
         constexpr std::size_t iters = 100000;
         auto eng = randEngine();
         std::uniform_int_distribution<std::uint64_t> dist;
+        std::uniform_int_distribution<std::uint64_t> dist1(1);
         for (int i = 0; i < iters; ++i)
         {
             std::uint64_t const d = dist(eng);
@@ -209,12 +210,31 @@ class base58_test : public beast::unit_test::suite
 
             auto const refAdd = boostBigInt + d;
 
-            b58_fast::detail::inplace_bigint_add(
+            auto const result = b58_fast::detail::inplace_bigint_add(
                 std::span<uint64_t>(bigInt.data(), bigInt.size()), d);
+            BEAST_EXPECT(result == TokenCodecErrc::success);
             auto const foundAdd = multiprecision_utils::toBoostMP(bigInt);
             BEAST_EXPECT(refAdd == foundAdd);
         }
         for (int i = 0; i < iters; ++i)
+        {
+            std::uint64_t const d = dist1(eng);
+            // Force overflow
+            std::vector<std::uint64_t> bigInt(
+                5, std::numeric_limits<std::uint64_t>::max());
+
+            auto const boostBigInt = multiprecision_utils::toBoostMP(
+                std::span<std::uint64_t>(bigInt.data(), bigInt.size()));
+
+            auto const refAdd = boostBigInt + d;
+
+            auto const result = b58_fast::detail::inplace_bigint_add(
+                std::span<uint64_t>(bigInt.data(), bigInt.size()), d);
+            BEAST_EXPECT(result == TokenCodecErrc::overflowAdd);
+            auto const foundAdd = multiprecision_utils::toBoostMP(bigInt);
+            BEAST_EXPECT(refAdd != foundAdd);
+        }
+        for (int i = 0; i < iters; ++i)
         {
             std::uint64_t const d = dist(eng);
             auto bigInt = multiprecision_utils::randomBigInt(/* minSize */ 2);
@@ -226,11 +246,29 @@ class base58_test : public beast::unit_test::suite
 
             auto const refMul = boostBigInt * d;
 
-            b58_fast::detail::inplace_bigint_mul(
+            auto const result = b58_fast::detail::inplace_bigint_mul(
                 std::span<uint64_t>(bigInt.data(), bigInt.size()), d);
+            BEAST_EXPECT(result == TokenCodecErrc::success);
             auto const foundMul = multiprecision_utils::toBoostMP(bigInt);
             BEAST_EXPECT(refMul == foundMul);
         }
+        for (int i = 0; i < iters; ++i)
+        {
+            std::uint64_t const d = dist1(eng);
+            // Force overflow
+            std::vector<std::uint64_t> bigInt(
+                5, std::numeric_limits<std::uint64_t>::max());
+            auto const boostBigInt = multiprecision_utils::toBoostMP(
+                std::span<std::uint64_t>(bigInt.data(), bigInt.size()));
+
+            auto const refMul = boostBigInt * d;
+
+            auto const result = b58_fast::detail::inplace_bigint_mul(
+                std::span<uint64_t>(bigInt.data(), bigInt.size()), d);
+            BEAST_EXPECT(result == TokenCodecErrc::inputTooLarge);
+            auto const foundMul = multiprecision_utils::toBoostMP(bigInt);
+            BEAST_EXPECT(refMul != foundMul);
+        }
     }
 
     void

Original file line number	Diff line number	Diff line change
`@@ -21,6 +21,7 @@`
`21`	`21`	`#define RIPPLE_PROTOCOL_B58_UTILS_H_INCLUDED`
`22`	`22`
`23`	`23`	`#include <ripple/basics/contract.h>`
	`24`	`+#include <ripple/protocol/impl/token_errors.h>`
`24`	`25`
`25`	`26`	`#include <boost/outcome.hpp>`
`26`	`27`	`#include <boost/outcome/result.hpp>`
`@@ -71,12 +72,12 @@ carrying_add(std::uint64_t a, std::uint64_t b)`
`71`	`72`	`// (i.e a[0] is the 2^0 coefficient, a[n] is the 2^(64*n) coefficient)`
`72`	`73`	`// panics if overflows (this is a specialized adder for b58 decoding.`
`73`	`74`	`// it should never overflow).`
`74`		`-inline void`
	`75`	`+[[nodiscard]] inline TokenCodecErrc`
`75`	`76`	`inplace_bigint_add(std::span<std::uint64_t> a, std::uint64_t b)`
`76`	`77`	`{`
`77`	`78`	`if (a.size() <= 1)`
`78`	`79`	`{`
`79`		`- ripple::LogicError("Input span too small for inplace_bigint_add");`
	`80`	`+ return TokenCodecErrc::inputTooSmall;`
`80`	`81`	`}`
`81`	`82`
`82`	`83`	`std::uint64_t carry;`
`@@ -86,28 +87,29 @@ inplace_bigint_add(std::span<std::uint64_t> a, std::uint64_t b)`
`86`	`87`	`{`
`87`	`88`	`if (!carry)`
`88`	`89`	`{`
`89`		`- return;`
	`90`	`+ return TokenCodecErrc::success;`
`90`	`91`	`}`
`91`	`92`	`std::tie(v, carry) = carrying_add(v, 1);`
`92`	`93`	`}`
`93`	`94`	`if (carry)`
`94`	`95`	`{`
`95`		`- LogicError("Overflow in inplace_bigint_add");`
	`96`	`+ return TokenCodecErrc::overflowAdd;`
`96`	`97`	`}`
	`98`	`+ return TokenCodecErrc::success;`
`97`	`99`	`}`
`98`	`100`
`99`		`-inline void`
	`101`	`+[[nodiscard]] inline TokenCodecErrc`
`100`	`102`	`inplace_bigint_mul(std::span<std::uint64_t> a, std::uint64_t b)`
`101`	`103`	`{`
`102`	`104`	`if (a.empty())`
`103`	`105`	`{`
`104`		`- LogicError("Empty span passed to inplace_bigint_mul");`
	`106`	`+ return TokenCodecErrc::inputTooSmall;`
`105`	`107`	`}`
`106`	`108`
`107`	`109`	`auto const last_index = a.size() - 1;`
`108`	`110`	`if (a[last_index] != 0)`
`109`	`111`	`{`
`110`		`- LogicError("Non-zero element in inplace_bigint_mul last index");`
	`112`	`+ return TokenCodecErrc::inputTooLarge;`
`111`	`113`	`}`
`112`	`114`
`113`	`115`	`std::uint64_t carry = 0;`
`@@ -116,7 +118,9 @@ inplace_bigint_mul(std::span<std::uint64_t> a, std::uint64_t b)`
`116`	`118`	`std::tie(coeff, carry) = carrying_mul(coeff, b, carry);`
`117`	`119`	`}`
`118`	`120`	`a[last_index] = carry;`
	`121`	`+ return TokenCodecErrc::success;`
`119`	`122`	`}`
	`123`	`+`
`120`	`124`	`// divide a "big uint" value inplace and return the mod`
`121`	`125`	`// numerator is stored so smallest coefficients come first`
`122`	`126`	`[[nodiscard]] inline std::uint64_t`
`@@ -166,11 +170,7 @@ inplace_bigint_div_rem(std::span<uint64_t> numerator, std::uint64_t divisor)`
`166`	`170`	`b58_10_to_b58_be(std::uint64_t input)`
`167`	`171`	`{`
`168`	`172`	`constexpr std::uint64_t B_58_10 = 430804206899405824; // 58^10;`
`169`		`- if (input >= B_58_10)`
`170`		`- {`
`171`		`- LogicError("Input to b58_10_to_b58_be equals or exceeds 58^10.");`
`172`		`- }`
`173`		`-`
	`173`	`+ assert(input < B_58_10);`
`174`	`174`	`constexpr std::size_t resultSize = 10;`
`175`	`175`	`std::array<std::uint8_t, resultSize> result{};`
`176`	`176`	`int i = 0;`
Original file line number	Diff line number	Diff line change
`@@ -32,6 +32,7 @@ enum class TokenCodecErrc {`
`32`	`32`	`mismatchedTokenType,`
`33`	`33`	`mismatchedChecksum,`
`34`	`34`	`invalidEncodingChar,`
	`35`	`+ overflowAdd,`
`35`	`36`	`unknown,`
`36`	`37`	`};`
`37`	`38`	`}`