Missing Input Validation Method Controls in Permit.sol

[c4-bot-4]

Lines of code

https://github.com/reserve-protocol/protocol/blob/master/contracts/libraries/Permit.sol#L17-L30

Vulnerability details

Impact

The function requireSignature() in the contract Pernit.sol does not properly validate the input that the contract receives.

Having insufficient checks of the owner variable means that there is potential for incorrect or malicious data to be processed by the contract.

This validates whether the provided owner address is a contract or not using the isContract call.

If it's a contract, it uses the isValidSignature call (which is standard defined in ERC1271) to check if the signature is valid.

But if the owner address is not a contract (which would typically indicate an EOA address), then it checks if the signature is valid using the isValidSignatureNow function.

There's a security concern here the contract only checks if it's an EOA or a contract address, but doesn't check if it's a valid EOA.

A malicious actor could potentially provide a false EOA.

Proof of Concept

https://github.com/reserve-protocol/protocol/blob/master/contracts/libraries/Permit.sol#L17-L30

if (AddressUpgradeable.isContract(owner)) {
    require(
        IERC1271Upgradeable(owner).isValidSignature(hash, abi.encodePacked(r, s, v)) ==
            0x1626ba7e,
        "ERC1271: Unauthorized"
    );
} else {
    require(
        SignatureCheckerUpgradeable.isValidSignatureNow(
            owner,
            hash,
            abi.encodePacked(r, s, v)
        ),
        "ERC20Permit: invalid signature"
    );
}

Tools Used

Manual Review

Recommended Mitigation Steps

A possible mitigation for this vulnerability is adding additional checks to validate the owner address more effectively.

This could involve using the EIP-55 to verify the checksum of an Ethereum address, or simply checking it against a whitelist of known valid addresses.

Assessed type

Invalid Validation