Biconomy - Smart Contract Wallet contest - Kutu's results

Lines of code

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

Vulnerability details

Impact

The function _requireFromEntryPointOrOwner() is being called within the execute() and executeBatch() functions to check if the msg.sender is either the owner or the EntryPoint contract, but these functions have onlyOwner() modifier, which will only allow the owner to execute these functions.

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

function executeBatch(address[] calldata dest, bytes[] calldata func) external onlyOwner{
    _requireFromEntryPointOrOwner();
    require(dest.length == func.length, "wrong array lengths");
    for (uint i = 0; i < dest.length;) {
        _call(dest[i], 0, func[i]);
        unchecked {
            ++i;
        }
    }
}

Proof of Concept

Calling these functions from the EntryPoint contract will fail

Tools Used

VSCode

Recommended Mitigation Steps

Remove onlyOwner modifier from both functions