LUSDToken Manual Review Findings

LUS-01M: Insecure Elliptic Curve Recovery Mechanism

Type	Severity	Location
Language Specific		LUSDToken.sol:L189

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
171function permit
172(
173    address owner, 
174    address spender, 
175    uint amount, 
176    uint deadline, 
177    uint8 v, 
178    bytes32 r, 
179    bytes32 s
180) 
181    external 
182    override 
183{            
184    require(deadline >= now, 'LUSD: expired deadline');
185    bytes32 digest = keccak256(abi.encodePacked('\x19\x01', 
186                     domainSeparator(), keccak256(abi.encode(
187                     _PERMIT_TYPEHASH, owner, spender, amount, 
188                     _nonces[owner]++, deadline))));
189    address recoveredAddress = ecrecover(digest, v, r, s);
190    require(recoveredAddress == owner, 'LUSD: invalid signature');
191    _approve(owner, spender, amount);
192}

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 (8bedd3b0df):

The LUSDToken::permit code was incorrectly adjusted to yield values when it fails rather than simply revert (i.e. via a require statement) which is a non-standard pattern. The LUSDToken::permit call should fatally fail if the input signature arguments are malleable. As such, we consider this exhibit partially alleviated.

Alleviation (e1d571bb78):

The code was updated to evaluate the s and v value validity via require checks, ensuring that the LUSDToken::permit function will revert if its input arguments are malformed. As such, we consider this exhibit fully alleviated.