code423n4 commented 1 year ago

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

c4-judge commented 1 year ago

gzeon-c4 marked the issue as duplicate of #390

c4-sponsor commented 1 year ago

livingrockrises marked the issue as sponsor confirmed