return value of 0 from ecrecover not checked

[code423n4]

Handle

gpersoon

Vulnerability details

Impact

The solidity function ecrecover is used, however the error result of 0 is not checked for. See documentation: https://docs.soliditylang.org/en/v0.8.9/units-and-global-variables.html?highlight=ecrecover#mathematical-and-cryptographic-functions "recover the address associated with the public key from elliptic curve signature or return zero on error. "

Now you can supply invalid input parameters to the Sig.recover function, which will then result 0. If you also set o.maker to be 0 then this will match and an invalid signature is not detected.

So you can do all kinds of illegal & unexpected transactions.

Proof of Concept

https://github.com/Swivel-Finance/gost/blob/v2/test/swivel/Swivel.sol#L476-L484 function validOrderHash(Hash.Order calldata o, Sig.Components calldata c) internal view returns (bytes32) { ... require(o.maker == Sig.recover(Hash.message(domain, hash), c), 'invalid signature'); return hash; }

https://github.com/Swivel-Finance/gost/blob/v2/test/swivel/Sig.sol#L16-L23 function recover(bytes32 h, Components calldata c) internal pure returns (address) { ... return ecrecover(h, c.v, c.r, c.s);

Tools Used

Recommended Mitigation Steps

Verify that the result from ecrecover isn't 0

[JTraversa]

Copy Pasting response from duplicate issue:

Id say this is noteable, but because all actions require approvals from o.maker, having 0x00 as o.maker with an "invalid" but valid signature should not be impactful. The suggestion would be to filter 0x00 makers from the orderbook? (which we do)