Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discrepancy in ecrecover #6966

Closed
2 tasks done
duncancmt opened this issue Jan 31, 2024 · 5 comments · Fixed by #6969
Closed
2 tasks done

Discrepancy in ecrecover #6966

duncancmt opened this issue Jan 31, 2024 · 5 comments · Fixed by #6969
Labels
T-bug Type: bug

Comments

@duncancmt
Copy link

Component

Forge

Have you ensured that all of these are up to date?

  • Foundry
  • Foundryup

What version of Foundry are you on?

forge 0.2.0 (a5171a4 2023-12-23T00:15:43.850312317Z) --- forge 0.2.0 (caef136 2024-01-29T00:20:25.410303708Z)

What command(s) is the bug in?

forge test

Operating System

Linux

Describe the bug

ecrecover has different behavior on recent forge (caef136) versus an older one (a5171a4). The discrepancy exists on at least solc 0.8.21 and 0.8.23.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8;

import "forge-std/Test.sol";

contract ECRecoverTest is Test {
    function testEcrecover() public {
        bytes32 h = 0x0000000000000000000000000000000000000000000000000000000000000000;
        uint8 v = 27;
        bytes32 r = bytes32(0xf87fff3202dfeae34ce9cb8151ce2e176bee02a937baac6de85c4ea03d6a6618);
        bytes32 s = bytes32(0xedf9ab5c7d3ec1df1c2b48600ab0a35f586e069e9a69c6cdeebc99920128d1a5);
        assertNotEq(ecrecover(h, v, r, s), address(0));
    }
}
$ foundryup -C a5171a4 ; echo '' >> test/ECRecover.t.sol ; forge test -vvvv --mt testEcrecover ; foundryup ; echo '' >> test/ECRecover.t.sol ; forge test -vvvv --mt testEcrecover

=== SNIP ===

foundryup: done
[⠆] Compiling...
[⠒] Compiling 25 files with 0.8.23
[⠰] Solc 0.8.23 finished in 3.78s
Compiler run successful!

Running 1 test for test/ECRecover.t.sol:ECRecoverTest
[PASS] testEcrecover() (gas: 3675)
Traces:
  [3675] ECRecoverTest::testEcrecover()
    ├─ [3000] PRECOMPILES::ecrecover(0x0000000000000000000000000000000000000000000000000000000000000000, 27, 112399737319495422990437385936927988781381678766989766053545082427252555802136, 107639272725494199625545639165460682519168644661731919450512484113299855167909) [staticcall]
    │   └─ ← 0x0000000000000000000000005e97e234ebf6c7d44e8b63d969906f9ca22cc886
    └─ ← ()

Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 471.46µs
 
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

=== SNIP ===

foundryup: done!
[⠢] Compiling...
[⠰] Compiling 1 files with 0.8.23
[⠔] Solc 0.8.23 finished in 1.14s
Compiler run successful!

Running 1 test for test/ECRecover.t.sol:ECRecoverTest
[FAIL. Reason: assertion failed] testEcrecover() (gas: 18217)
Logs:
  Error: a != b not satisfied [address]
        Left: 0x0000000000000000000000000000000000000000
       Right: 0x0000000000000000000000000000000000000000

Traces:
  [18217] ECRecoverTest::testEcrecover()
    ├─ [3000] PRECOMPILES::ecrecover(0x0000000000000000000000000000000000000000000000000000000000000000, 27, 112399737319495422990437385936927988781381678766989766053545082427252555802136, 107639272725494199625545639165460682519168644661731919450512484113299855167909) [staticcall]
    │   └─ ← ()
    ├─ emit log(val: "Error: a != b not satisfied [address]")
    ├─ emit log_named_address(key: "      Left", val: 0x0000000000000000000000000000000000000000)
    ├─ emit log_named_address(key: "     Right", val: 0x0000000000000000000000000000000000000000)
    ├─ [0] VM::store(VM: [0x7109709ECfa91a80626fF3989D68f67F5b1DD12D], 0x6661696c65640000000000000000000000000000000000000000000000000000, 0x0000000000000000000000000000000000000000000000000000000000000001)
    │   └─ ← ()
    └─ ← ()

Test result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 425.26µs
 
Ran 1 test suites: 0 tests passed, 1 failed, 0 skipped (1 total tests)

Failing tests:
Encountered 1 failing test in test/ECRecover.t.sol:ECRecoverTest
[FAIL. Reason: assertion failed] testEcrecover() (gas: 18217)

Encountered a total of 1 failing tests, 0 tests succeeded
@DaniPopes
Copy link
Member

DaniPopes commented Jan 31, 2024

Bisected to k256 update in #6912.
k256 diff https://diff.rs/k256/0.13.1/0.13.2/
k256 changelog https://github.com/RustCrypto/elliptic-curves/blob/master/k256/CHANGELOG.md#0132-2023-11-15

Patching k256 = "=0.13.1" with

diff --git a/Cargo.lock b/Cargo.lock
index a51c4e0d..c90fa32f 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -4425,9 +4425,9 @@ dependencies = [
 
 [[package]]
 name = "k256"
-version = "0.13.3"
+version = "0.13.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "956ff9b67e26e1a6a866cb758f12c6f8746208489e3e4a4b5580802f2f0a587b"
+checksum = "cadb76004ed8e97623117f3df85b17aaa6626ab0b0831e6573f104df16cd1bcc"
 dependencies = [
  "cfg-if",
  "ecdsa",
diff --git a/Cargo.toml b/Cargo.toml
index 8ab5b54c..99f6c60e 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -190,7 +190,7 @@ toml = "0.8"
 tracing = "0.1"
 tracing-subscriber = "0.3"
 evm-disassembler = "0.4"
-k256 = "0.13"
+k256 = "=0.13.1"
 
 axum = "0.6"
 hyper = "0.14"

Makes it pass again:

$ forge2 -V
forge 0.2.0 (888d349 2024-01-31T17:12:06.315793834Z)
$ forge t
[⠆] Compiling...
No files changed, compilation skipped

Running 1 test for test/Counter.t.sol:ECRecoverTest
[PASS] testEcrecover() (gas: 3675)
Traces:
  [3675] ECRecoverTest::testEcrecover()
    ├─ [3000] PRECOMPILES::ecrecover(0x0000000000000000000000000000000000000000000000000000000000000000, 27, 112399737319495422990437385936927988781381678766989766053545082427252555802136, 107639272725494199625545639165460682519168644661731919450512484113299855167909) [staticcall]
    │   └─ ← 0x0000000000000000000000005e97e234ebf6c7d44e8b63d969906f9ca22cc886
    └─  ()

Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 3.06ms
 
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

@duncancmt
Copy link
Author

Ahh

Reject signatures which aren't low-S normalized (RustCrypto/elliptic-curves#914)

@DaniPopes
Copy link
Member

Related RustCrypto/elliptic-curves#988

Where did you get that signature from?

@duncancmt
Copy link
Author

It's not a real signature. I'm abusing ecrecover to get secp256k1 ecmul https://ethresear.ch/t/you-can-kinda-abuse-ecrecover-to-do-ecmul-in-secp256k1-today/2384 . The specific constants came from a failing fuzz test.

@DaniPopes
Copy link
Member

OK, we should pin k256 until revm fix is released: bluealloy/revm#870

@github-project-automation github-project-automation bot moved this from Todo to Done in Foundry Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bug Type: bug
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants