There are no checks to ensure ecrecover() does not return 0.

Summary

return value of 0 from ecrecover not checked

Vulnerability Detail

The solidity function ecrecover is used in UniversalSigValidator.sol and function isValidSigImpl, 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. "

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

As such, any invalid signature would be treated as valid when paired with a zero address.

Impact

invalid signature can then be used to authenticate transactions.

Code Snippet

        // ecrecover verification
        require(_signature.length == 65, "SignatureValidator#recoverSigner: invalid signature length");
        bytes32 r = bytes32(_signature[0:32]);
        bytes32 s = bytes32(_signature[32:64]);
        uint8 v = uint8(_signature[64]);
        if (v != 27 && v != 28) {
            revert("SignatureValidator: invalid signature v value");
        }
        return ecrecover(_hash, v, r, s) == _signer;
    }

https://github.com/sherlock-audit/2024-02-perpetual/blob/main/perp-contract-v3/src/external/universalSigValidator/UniversalSigValidator.sol#L18

Tool used

Manual Review

Recommendation

Verify that the result from ecrecover isn't 0 also we strongly recommend considering the use of OpenZeppelin’s ECDSA library: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/cryptography/ECDSA.sol

sherlock-audit / 2024-02-perpetual-judging

Avci - There are no checks to ensure ecrecover() does not return 0. #142