diff --git a/lib/evmone_precompiles/ecc.hpp b/lib/evmone_precompiles/ecc.hpp index 206831b89e..f322dca69d 100644 --- a/lib/evmone_precompiles/ecc.hpp +++ b/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 static constexpr std::optional from_bytes( std::span b) noexcept { + constexpr auto LIMIT = R == Range::full ? ORDER : ORDER / 2 + 1; + const auto x = intx::be::load(b); - if (x >= ORDER) [[unlikely]] + if (x >= LIMIT) [[unlikely]] return std::nullopt; return FieldElement{x}; } diff --git a/lib/evmone_precompiles/secp256k1.cpp b/lib/evmone_precompiles/secp256k1.cpp index cd64164daa..11b77587b2 100644 --- a/lib/evmone_precompiles/secp256k1.cpp +++ b/lib/evmone_precompiles/secp256k1.cpp @@ -45,8 +45,8 @@ evmc::address to_address(const AffinePoint& pt) noexcept } std::optional secp256k1_ecdsa_recover(std::span hash, - std::span r_bytes, std::span s_bytes, - bool parity) noexcept + std::span r_bytes, std::span 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 secp256k1_ecdsa_recover(std::span 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(s_bytes) : + Curve::Fr::from_bytes(s_bytes); if (!opt_s.has_value() || *opt_s == 0) [[unlikely]] return std::nullopt; @@ -93,11 +95,11 @@ std::optional secp256k1_ecdsa_recover(std::span } std::optional ecrecover(std::span hash, - std::span r_bytes, std::span s_bytes, - bool parity) noexcept + std::span r_bytes, std::span 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; diff --git a/lib/evmone_precompiles/secp256k1.hpp b/lib/evmone_precompiles/secp256k1.hpp index 0bd6ddb1a5..08b5108fe1 100644 --- a/lib/evmone_precompiles/secp256k1.hpp +++ b/lib/evmone_precompiles/secp256k1.hpp @@ -57,12 +57,22 @@ evmc::address to_address(std::span 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 secp256k1_ecdsa_recover(std::span hash, - std::span r_bytes, std::span s_bytes, - bool parity) noexcept; + std::span r_bytes, std::span s_bytes, bool parity, + RecoveryMode mode) noexcept; +/// Recovers the address that signed the message @p hash. +/// +/// TODO: Make strict mode the default. std::optional ecrecover(std::span hash, - std::span r_bytes, std::span s_bytes, - bool parity) noexcept; + std::span r_bytes, std::span s_bytes, bool parity, + RecoveryMode mode = RecoveryMode::malleable) noexcept; } // namespace evmmax::secp256k1 diff --git a/test/unittests/evmmax_secp256k1_test.cpp b/test/unittests/evmmax_secp256k1_test.cpp index 6619570981..79e5178000 100644 --- a/test/unittests/evmmax_secp256k1_test.cpp +++ b/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 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(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); + } + } +}