code-423n4 / 2023-01-biconomy-findings

12 stars 10 forks source link

Signature Replay Attack when EntryPoint contract is changed #469

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-biconomy/blob/main/scw-contracts/contracts/smart-contract-wallet/paymasters/verifying/singleton/VerifyingSingletonPaymaster.sol#L77-L90

Vulnerability details

Signature Replay Attack when EntryPoint contract is changed

Impact

User operations can be replayed on smart accounts once the EntryPoint is changed. This can lead to user's loosing funds or any unexpected behaviour that transaction replay attacks usually lead to.

Proof of Concept

As specified by the EIP4337 standard to prevent replay attacks (both cross-chain and multiple EntryPoint implementations), the signature should depend on chainid and the EntryPoint address.. In VerifyingSingletonPaymaster.sol#getHash the EntryPoint contract address is missing which means that the same UserOperation can be replayed on the same smart contract account once the EntryPoint is changed.

Tools Used

Manual review

Recommended Mitigation Steps

Add the EntryPoint contract address in the calculation of the UserOperation hash in VerifyingSingletonPaymaster.sol#getHash

    function getHash(UserOperation calldata userOp)
    public view returns (bytes32) { // @audit change to view
        //can't use userOp.hash(), since it contains also the paymasterAndData itself.
        return keccak256(abi.encode(
                userOp.getSender(),
                userOp.nonce,
                keccak256(userOp.initCode),
                keccak256(userOp.callData),
                userOp.callGasLimit,
                userOp.verificationGasLimit,
                userOp.preVerificationGas,
                userOp.maxFeePerGas,
                userOp.maxPriorityFeePerGas
        address(entryPoint) // @audit add entry point contract address (inherited from BasePaymaster)
            ));
    }
c4-judge commented 1 year ago

gzeon-c4 marked the issue as primary issue

c4-sponsor commented 1 year ago

livingrockrises marked the issue as sponsor disputed

livingrockrises commented 1 year ago

same userop, same smart account and different entry point will still fail signature validation.

livingrockrises commented 1 year ago

        return keccak256(abi.encode(userOp.hash(), address(this), block.chainid));
    }```
c4-judge commented 1 year ago

gzeon-c4 marked the issue as unsatisfactory: Invalid