crypto: Add non-malleable secp256k1 ecrecover mode

lib/evmone_precompiles/ecc.hpp

@@ -52,11 +52,21 @@ class FieldElement
 
     constexpr uint_type value() const noexcept { return Fp.from_mont(value_); }
 
+    /// The valid range for from_bytes().
+    enum class Range : bool
+    {
+        full,  ///< Valid in [0, ORDER).
+        half,  ///< Valid in [0, ORDER/2].
+    };
+
+    template <Range R = Range::full>
     static constexpr std::optional<FieldElement> from_bytes(
         std::span<const uint8_t, sizeof(uint_type)> b) noexcept
     {
+        constexpr auto LIMIT = R == Range::full ? ORDER : ORDER / 2 + 1;
+
         const auto x = intx::be::load<uint_type>(b);
-        if (x >= ORDER) [[unlikely]]
+        if (x >= LIMIT) [[unlikely]]
             return std::nullopt;
         return FieldElement{x};
     }

lib/evmone_precompiles/secp256k1.cpp

@@ -45,8 +45,8 @@ evmc::address to_address(const AffinePoint& pt) noexcept
 }
 
 std::optional<AffinePoint> secp256k1_ecdsa_recover(std::span<const uint8_t, 32> hash,
-    std::span<const uint8_t, 32> r_bytes, std::span<const uint8_t, 32> s_bytes,
-    bool parity) noexcept
+    std::span<const uint8_t, 32> r_bytes, std::span<const uint8_t, 32> s_bytes, bool parity,
+    RecoveryMode mode) noexcept
 {
     // Follows "Elliptic Curve Digital Signature Algorithm - Public key recovery"
     // https://en.wikipedia.org/wiki/Elliptic_Curve_Digital_Signature_Algorithm#Public_key_recovery
@@ -56,7 +56,9 @@ std::optional<AffinePoint> secp256k1_ecdsa_recover(std::span<const uint8_t, 32>
     if (!opt_r.has_value() || *opt_r == 0) [[unlikely]]
         return std::nullopt;
 
-    const auto opt_s = Curve::Fr::from_bytes(s_bytes);
+    const auto opt_s = mode == RecoveryMode::strict ?
+                           Curve::Fr::from_bytes<Curve::Fr::Range::half>(s_bytes) :
+                           Curve::Fr::from_bytes<Curve::Fr::Range::full>(s_bytes);
     if (!opt_s.has_value() || *opt_s == 0) [[unlikely]]
         return std::nullopt;
 
@@ -93,11 +95,11 @@ std::optional<AffinePoint> secp256k1_ecdsa_recover(std::span<const uint8_t, 32>
 }
 
 std::optional<evmc::address> ecrecover(std::span<const uint8_t, 32> hash,
-    std::span<const uint8_t, 32> r_bytes, std::span<const uint8_t, 32> s_bytes,
-    bool parity) noexcept
+    std::span<const uint8_t, 32> r_bytes, std::span<const uint8_t, 32> s_bytes, bool parity,
+    RecoveryMode mode) noexcept
 {
     // TODO(C++23): use std::optional::and_then.
-    const auto pubkey = secp256k1_ecdsa_recover(hash, r_bytes, s_bytes, parity);
+    const auto pubkey = secp256k1_ecdsa_recover(hash, r_bytes, s_bytes, parity, mode);
     if (!pubkey.has_value())
         return std::nullopt;
 

lib/evmone_precompiles/secp256k1.hpp

@@ -57,12 +57,22 @@ evmc::address to_address(std::span<const uint8_t, 64> pubkey) noexcept;
 /// Convert the secp256k1 point (uncompressed public key) to Ethereum address.
 evmc::address to_address(const AffinePoint& pt) noexcept;
 
+/// The strictness of the signer recovery from a signature.
+enum class RecoveryMode : bool
+{
+    strict,     ///< Restrict s value range to lower half, prevents signature malleability (EIP-2).
+    malleable,  ///< Full range for s value, signature is malleable.
+};
+
 std::optional<AffinePoint> secp256k1_ecdsa_recover(std::span<const uint8_t, 32> hash,
-    std::span<const uint8_t, 32> r_bytes, std::span<const uint8_t, 32> s_bytes,
-    bool parity) noexcept;
+    std::span<const uint8_t, 32> r_bytes, std::span<const uint8_t, 32> s_bytes, bool parity,
+    RecoveryMode mode) noexcept;
 
+/// Recovers the address that signed the message @p hash.
+///
+/// TODO: Make strict mode the default.
 std::optional<evmc::address> ecrecover(std::span<const uint8_t, 32> hash,
-    std::span<const uint8_t, 32> r_bytes, std::span<const uint8_t, 32> s_bytes,
-    bool parity) noexcept;
+    std::span<const uint8_t, 32> r_bytes, std::span<const uint8_t, 32> s_bytes, bool parity,
+    RecoveryMode mode = RecoveryMode::malleable) noexcept;
 
 }  // namespace evmmax::secp256k1

test/unittests/evmmax_secp256k1_test.cpp

@@ -293,6 +293,12 @@ const TestCase TEST_CASES[]{
     // R == 2G, high s
     {"ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff 000000000000000000000000000000000000000000000000000000000000001c c6047f9441ed7d6d3045406e95c07cd85c778e4b8cef3ca7abac09b95c709ee5 fffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd036413b",
         "000000000000000000000000bbb10a3b5835400b63ca00372c16db781220fb0b"},
+    // R == 2G, s == ORDER/2: the highest s a strict (EIP-2) recovery accepts.
+    {"ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff 000000000000000000000000000000000000000000000000000000000000001c c6047f9441ed7d6d3045406e95c07cd85c778e4b8cef3ca7abac09b95c709ee5 7fffffffffffffffffffffffffffffff5d576e7357a4501ddfe92f46681b20a0",
+        "00000000000000000000000090dd1d3d5a9814647c17016ce932360f61639baa"},
+    // R == 2G, s == ORDER/2 + 1: the lowest s a strict recovery rejects.
+    {"ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff 000000000000000000000000000000000000000000000000000000000000001c c6047f9441ed7d6d3045406e95c07cd85c778e4b8cef3ca7abac09b95c709ee5 7fffffffffffffffffffffffffffffff5d576e7357a4501ddfe92f46681b20a1",
+        "00000000000000000000000026944cf58be26228fdf1e153c37e2152a21a7a97"},
     // R == 3G, low s
     {"ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff 000000000000000000000000000000000000000000000000000000000000001c f9308a019258c31049344f85f89d5229b531c845836f99b08601f113bce036f9 0000000000000000000000000000000000000000000000000000000000000010",
         "000000000000000000000000620833dce54ca9329f13a22c3831b102f15df27c"},
@@ -309,7 +315,7 @@ const TestCase TEST_CASES[]{
 };
 }  // namespace
 
-TEST(evmmax, ecrecovery)
+TEST(evmmax, ecrecovery_malleable)
 {
     for (const auto& [input_hex, expected_output_hex] : TEST_CASES)
     {
@@ -339,3 +345,42 @@ TEST(evmmax, ecrecovery)
         }
     }
 }
+
+TEST(evmmax, ecrecovery_strict)
+{
+    const auto order_half = "7fffffffffffffffffffffffffffffff5d576e7357a4501ddfe92f46681b20a0"_hex;
+    ASSERT_EQ(order_half.size(), 32);
+
+    for (const auto& [input_hex, malleable_expected_output_hex] : TEST_CASES)
+    {
+        const auto input = from_spaced_hex(input_hex).value();
+        ASSERT_EQ(input.size(), 128);
+
+        const std::span<const uint8_t, 128> input_span{input};
+        const auto hash = input_span.subspan<0, 32>();
+        const auto v_bytes = input_span.subspan<32, 32>();
+        const auto r_bytes = input_span.subspan<64, 32>();
+        const auto s_bytes = input_span.subspan<96, 32>();
+
+        // Both are 32-byte big-endian values, so the byte-wise order is the numeric one.
+        const auto s_high = std::ranges::lexicographical_compare(order_half, s_bytes);
+        const auto expected_output_hex =
+            !s_high ? malleable_expected_output_hex : std::string_view{};
+
+        const auto v = be::unsafe::load<uint256>(v_bytes.data());
+        ASSERT_TRUE(v == 27 || v == 28);
+        const bool parity = v == 28;
+
+        const auto result = ecrecover(hash, r_bytes, s_bytes, parity, RecoveryMode::strict);
+
+        if (expected_output_hex.empty())
+        {
+            EXPECT_FALSE(result.has_value());
+        }
+        else
+        {
+            ASSERT_TRUE(result.has_value());
+            EXPECT_EQ(std::string(24, '0') + hex(*result), expected_output_hex);
+        }
+    }
+}