owned this note
owned this note
Published
Linked with GitHub
# RIP-7212 Implementation Issue
## ETH Magician
Source: https://ethereum-magicians.org/t/eip-7212-precompiled-for-secp256r1-curve-support/14789?page=5
### Discussion
- People argue that secp256r1 is not a safe ECC compared to ed25519
- Rumor has it that NSA [control](https://ethereum-magicians.org/t/eip-7212-precompiled-for-secp256r1-curve-support/14789/12) the seed for the algorithm (?
- Big [tech](https://ethereum-magicians.org/t/eip-7212-precompiled-for-secp256r1-curve-support/14789/16) still use secp256r1 curve, and there is no strong evidence for the accusation.
- [Interface issue](https://ethereum-magicians.org/t/eip-7212-precompiled-for-secp256r1-curve-support/14789/95) for invalid singature
- ZkSync returns uint(0), while Polygon returns empty data
- Root cause: EIP does not clearly specify the return value
- Impact: Dev might be confused whether the failure result from nonexistent precompile or invalid signature
## Audit Finding
### Mantle
Link: https://blog.openzeppelin.com/mantle-op-geth-op-stack-diff-audit#missing-signature-malleability-check
> [Info] Insufficient documentation for signature malleability check
**Descrption**: Missing Signature Malleability Check
ECDSA signatures are inherently malleable because both `(r,s)` and
`(r,−s)` are valid for the same message. To prevent this, implementations usually enforce 𝑠 ≤ 𝑁 /2, where 𝑁 is the curve order. However, the `p256Verify` precompile intentionally omits this malleability check to align with the NIST FIPS 186-5 specification, as stated in RIP-7212. The standard recommends that wrapper libraries should include the malleability check by default and clearly differentiate between functions that do or do not enforce it.
**Solution** The Mantle team has explicitly documented this design decision, citing RIP-7212 as the rationale for omitting the check in the precompile.
> [Info] Improved Efficiency of Input Validation
**Description**: The newPublicKey function ensures that the provided
(𝑥,𝑦) point:
- Is not nil, nil
- Lies on the P256 curve (checked via IsOnCurve(x, y))
Although it doesn't explicitly reject the point-at-infinity
(0,0), this case is implicitly handled since
(0,0) is not on the curve and is rejected by IsOnCurve.
However, IsOnCurve is computationally expensive. To improve efficiency, it's suggested to add a preliminary range check on
(𝑥,𝑦) (similar to ecrecover), verifying byte length and that the values fall within the curve’s order 𝑁. This would avoid unnecessary expensive calls to IsOnCurve for clearly invalid inputs.
**Trade-off**
Such a check introduces redundancy because IsOnCurve already includes these validations. Therefore, the benefit depends on how often IsOnCurve is called. Alternatively, modifying IsOnCurve to skip redundant checks could improve performance.
**Recommendations**:
- Consider adding a lightweight range check before calling IsOnCurve
- Explicitly reject or document the implicit rejection of (0,0) for clarity
### kakarot
Link: https://github.com/code-423n4/2024-09-kakarot-findings/issues/58
> [Info] p256verify precompile charges new account access gas
The Kakarot EVM correctly keeps precompile addresses warm, with the goal of charging reduced "warm address access" gas for precompile calls.
Because the 0x100 address is not cached, when the p256verify is called, the caller will be charged 2500 extra gas.
This is also the case for the Kakarot cairo_precompile and cairo_message precompiles, however we report this impact with lower severity because unlike with p256verify there is no formal specification for these precompiles.
## Test Cases
I examine all the client that uses the Wycheproof as test case, try to find out if there are additional cases added.
| Project | Link | Test Case Number |
|-----------|---------|------------------|
| Wycheproof | [Link](https://github.com/C2SP/wycheproof/blob/main/testvectors/ecdsa_secp256r1_sha256_test.json) | 387 |
| Go-eth | [Link](https://github.com/ethereum/go-ethereum/pull/27540) | 781 |
| Op-Geth | [Link](https://github.com/mdehoog/op-geth/blob/1aa4e22e506592ea22a3cc1e5f9e785208524ebc/core/vm/testdata/precompiles/p256Verify.json) | 781 |
| Polygon | [Link](https://github.com/erigontech/erigon/pull/8975/files) | 5 (???) |
| Scroll | [Link](https://github.com/scroll-tech/go-ethereum/blob/91262060b77d99303c2af9bf93f5daadc8cab558/core/vm/testdata/precompiles/p256Verify.json) | 781 |
| Daimo | [Link](https://github.com/daimo-eth/p256-verifier/blob/master/test-vectors/vectors_wycheproof.jsonl) | 778 |