_validateSignature should not revert on invalid signature (EIP-4337)

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-biconomy/blob/main/scw-contracts/contracts/smart-contract-wallet/SmartAccount.sol#L511

Vulnerability details

Impact

Results in unexpected behavior in the EntryPoint contract.

Proof of Concept

As said in the official specification of EIP-4337: "If the account does not support signature aggregation, it MUST validate the signature is a valid signature of the userOpHash, and SHOULD return SIG_VALIDATION_FAILED (and not revert) on signature mismatch. Any other error should revert.". SmartAccount._validateSignature does the opposite and reverts if the signature is invalid which will be wrong interpret by the EntryPoint contract.

Tools Used

Manual review

Recommended Mitigation Steps

Return SIG_VALIDATION_FAILED instead of revert in SmartAccount._validateSignature:

    function _validateSignature(UserOperation calldata userOp, bytes32 userOpHash, address)
    internal override virtual returns (uint256 deadline) {
        bytes32 hash = userOpHash.toEthSignedMessageHash();
        //ignore signature mismatch of from==ZERO_ADDRESS (for eth_callUserOp validation purposes)
        // solhint-disable-next-line avoid-tx-origin
-       require(owner == hash.recover(userOp.signature) || tx.origin == address(0), "account: wrong signature");
+       if(owner != hash.recover(userOp.signature) && tx.origin != address(0)) return SIG_VALIDATION_FAILED;
        return 0;
    }

[c4-judge]

gzeon-c4 marked the issue as duplicate of #318

[livingrockrises]

you're right. it has to be rebased with the latest code (0.4.0).

[c4-sponsor]

livingrockrises marked the issue as sponsor confirmed

[c4-judge]

gzeon-c4 marked the issue as satisfactory

[c4-judge]

gzeon-c4 marked the issue as duplicate of #498