ERC20Permit Manual Review Findings

ERC-01M: Insecure EIP-2612 Implementation

Type	Severity	Location
Logical Fault		ERC20Permit.sol:L61-L78

Description:

The ERC20Permit contract will calculate the DOMAIN_SEPARATOR only once during its lifetime within its constructor.

Impact:

While the likelihood of a blockchain fork resulting in a viable chain is very low, the attack vector is trivially exploitable should this happen and would cause fund loss.

Example:

Copy
61constructor() {
62    uint256 chainID;
63    assembly {
64        chainID := chainid()
65    }
66
67    DOMAIN_SEPARATOR = keccak256(
68        abi.encode(
69            keccak256(
70                "EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"
71            ),
72            keccak256(bytes(name())),
73            keccak256(bytes("1")), // Version
74            chainID,
75            address(this)
76        )
77    );
78}

Recommendation:

We strongly advise a paradigm similar to OpenZeppelin's draft-ERC20Permit to be applied, re-calculating the DOMAIN_SEPARATOR with the current chainid on a need-to basis as the contract is currently susceptible to cross-chain replay attacks should the blockchain it is deployed into be forked.

Alleviation:

The Gravita Protocol team has opted to not apply a remediation for this exhibit thus rendering it acknowledged.

ERC-02M: Insecure Elliptic Curve Recovery Mechanism

Type	Severity	Location
Language Specific		ERC20Permit.sol:L101

Description:

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).

Impact:

Should the payload being verified by the signature rely on differentiation based on the s or v arguments, it will be possible to replay the signature for the same data validly and acquire authorization twice.

Example:

Copy
84function permit(
85    address owner,
86    address spender,
87    uint256 amount,
88    uint256 deadline,
89    uint8 v,
90    bytes32 r,
91    bytes32 s
92) public virtual override {
93    require(block.timestamp <= deadline, "Permit: expired deadline");
94
95    bytes32 hashStruct = keccak256(
96        abi.encode(PERMIT_TYPEHASH, owner, spender, amount, _nonces[owner].current(), deadline)
97    );
98
99    bytes32 _hash = keccak256(abi.encodePacked(uint16(0x1901), DOMAIN_SEPARATOR, hashStruct));
100
101    address signer = ecrecover(_hash, v, r, s);
102    require(signer != address(0) && signer == owner, "ERC20Permit: Invalid signature");
103
104    _nonces[owner].increment();
105    _approve(owner, spender, amount);
106}

Recommendation:

We 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.

Alleviation:

The ECDSA library of OpenZeppelin is now in use by the codebase that applies the relevant security checks, alleviating this exhibit.