The Ecrecover.yul file meant to simulate the ecrecover mechanism as executed by traditional ETH 1.0 consensus mechanisms is incorrect. In detail, it does not conform to the βHomesteadβ update which introduced an upper-bound check for s values of an (r, s, v) ECDSA payload to prohibit malleable signatures.
All major ETH 1.0 clients (i.e. go-ethereum, nethermind) prevent s values as input in ecrecover instructions if the value lies in the upper half of the secp256k1 curve order. To illustrate:
The consensus discrepancy at hand illustrates a security feature present in ETH 1.0 clients and absent in the zkEVM. As such, a contract that would be secure when deployed in ETH 1.0 will not be so when deployed in the zkEVM. This vulnerability allows malleable signatures to be consumed thus opening up potential replay-attack vectors that would otherwise be impossible in ETH 1.0.
The ecrecover function lies at the heart of multiple authorization systems in use by smart contracts, meaning that a significant amount of funds would be affected by this discrepancy.
An ecrecover call with an s value that is greater than or equal to 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A1 should fail execution, however, the operation properly succeeds in the zkEVM. A minimalistic contract that illustrates the issue:
contract ECRecoverVulnerability {
function test(uint8 v, uint256 r, uint256 s) external view {
require(s >= 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A1, "S value be in upper-half of secp256k1 curve order");
ecrecover(r, s, v);
revert("ISSUE: Should not have reached here");
}
}
Manual review, ETH 1.0 clients, Ethereum Foundation release guidelines.
We advise the upper bound evaluation to be performed similarly to the SECP256K1_GROUP_SIZE bound check already applied to the codebase, utilizing the nethermind coding style which divides the group size by 2.
The text was updated successfully, but these errors were encountered:
All reactions