Biconomy - Smart Contract Wallet contest - 0xDave's results

Lines of code

https://github.com/code-423n4/2023-01-biconomy/blob/53c8c3823175aeb26dee5529eeefa81240a406ba/scw-contracts/contracts/smart-contract-wallet/SmartAccount.sol#L489-L492 https://github.com/code-423n4/2023-01-biconomy/blob/53c8c3823175aeb26dee5529eeefa81240a406ba/scw-contracts/contracts/smart-contract-wallet/SmartAccount.sol#L460

Vulnerability details

    function execute(address dest, uint value, bytes calldata func) external onlyOwner{
        _requireFromEntryPointOrOwner();
        _call(dest, value, func);
    }

    function execFromEntryPoint(address dest, uint value, bytes calldata func, Enum.Operation operation, uint256 gasLimit) external onlyEntryPoint returns (bool success) {        
        success = execute(dest, value, func, operation, gasLimit);
        require(success, "Userop Failed");
    }

execFromEntryPoint is a function that can only be accessed by entrypoint. When entrypoint call the function, execute is called. execute, however, can only be called by the owner. As the function has _requireFromEntryPointOrOwner(), it seems that entrypoint can also be called, but execute is using the onlyOwner modifier, so it cannot be accessed unless it is an owner.

Tools Used

Manual audit

Recommended Mitigation Steps

execute should be modified to use the mixedAuth modifier as follows. Even if we delete _requireFromEntryPointOrOwner(); the owner can be accessed via mixedAuth.

    function execute(address dest, uint value, bytes calldata func) external mixedAuth{
        _call(dest, value, func);
    }