Insecure Elliptic Curve Recovery Mechanism which may permit signature replays.

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/main/src%2FSigningTools.sol#L26

Vulnerability details

Impact

The ecrecover() function is a low-level cryptographic function that should be utilized after appropriate sanitizations have been enforced on its arguments, namely on the s and v values. This is due to the inherent trait of the curve to be symmetrical on the x-axis and thus permitting signatures to be replayed with the same x value (r) but a different y value (s).

• https://github.com/code-423n4/2024-01-salty/blob/main/src%2FSigningTools.sol#L26

                address recoveredAddress = ecrecover(messageHash, v, r, s);

Proof of Concept

Here's a simple proof of concept demonstrating this vulnerability.

pragma solidity ^0.8.0;

contract Test {
    function testRecover(bytes32 hash, uint8 v, bytes32 r, bytes32 s) public pure returns (address) {
        return ecrecover(hash, v, r, s);
    }
}

In this contract, the testRecover() function takes a hash and a signature as parameters and returns the address derived from the signature. An attacker could call this function with a different signature that corresponds to the same hash, effectively impersonating the original signer.

It's important to note that this vulnerability can lead to serious security issues, especially in scenarios where signatures are used for authentication or identification purposes, such as in decentralized identity systems, non-fungible tokens (NFTs), multi-signature wallets, and decentralized finance (DeFi) applications.

Tools Used

Manual Review VS code

Recommended Mitigation Steps

I advise them to be sanitized by ensuring that v is equal to either 27 or 28 (v ∈ {27, 28}) and to ensure that s is existent in the lower half order of the elliptic curve (0 < s < secp256k1n ÷ 2 + 1) by ensuring it is less than 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A1. A reference implementation of those checks can be observed in the ECDSA library of OpenZeppelin and the rationale behind those restrictions exists within Appendix F of the Yellow Paper.

Alternatively, use the latest EIP-712 standard, which provides a more secure way of handling signatures in Solidity. This standard includes a structured data argument that can be hashed and signed, providing better protection against replay attacks and signature malleability.

Assessed type

Other

[c4-judge]

Picodes marked the issue as duplicate of #403

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)