Biconomy - Smart Contract Wallet contest - hansfriese's results

Lines of code

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

Vulnerability details

Impact

Entrypoint can not call the functions execute() and executeBatch().

Proof of Concept

There are two functions in SmartAccount contract related to execution implemented as below.

SmartAccount.sol
460:     function execute(address dest, uint value, bytes calldata func) external onlyOwner{
461:         _requireFromEntryPointOrOwner();
462:         _call(dest, value, func);
463:     }
464:
465:     function executeBatch(address[] calldata dest, bytes[] calldata func) external onlyOwner{
466:         _requireFromEntryPointOrOwner();
467:         require(dest.length == func.length, "wrong array lengths");
468:         for (uint i = 0; i < dest.length;) {
469:             _call(dest[i], 0, func[i]);
470:             unchecked {
471:                 ++i;
472:             }
473:         }
474:     }

These two functions are supposed to be callable by the EntryPoint contract and the function _requireFromEntryPointOrOwner() is used as a guard for caller. But the functions have the modifier onlyOwner and it reverts when these are called by the EntryPoint contract.

Tools Used

Manual Review

Recommended Mitigation Steps

Remove the onlyOnwer modifier for the functions execute() and executeBatch().