Low-level call/delegatecall can fail silently

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-biconomy/blob/53c8c3823175aeb26dee5529eeefa81240a406ba/scw-contracts/contracts/smart-contract-wallet/base/Executor.sol#L13-L34

Vulnerability details

Impact

execute function in Executor.sol will return true when calling non-existent contracts. As stated in the solidity docs "The low-level functions call, delegatecall and staticcall return true as their first return value if the account called is non-existent, as part of the design of the EVM."

function execute(
    address to,
    uint256 value,
    bytes memory data,
    Enum.Operation operation,
    uint256 txGas
) internal returns (bool success) { 
    if (operation == Enum.Operation.DelegateCall) {
        // solhint-disable-next-line no-inline-assembly
        assembly {
            success := delegatecall(txGas, to, add(data, 0x20), mload(data), 0, 0)
        }
    } else {
        // solhint-disable-next-line no-inline-assembly
        assembly {
            success := call(txGas, to, value, add(data, 0x20), mload(data), 0, 0) 
        }
    }
    // Emit events here..
    if (success) emit ExecutionSuccess(to, value, data, operation, txGas);
    else emit ExecutionFailure(to, value, data, operation, txGas);
}

There is no check for contract existence before calling execute, nor inside execute function. Therefore this function can fail silently.

Recommended Mitigation Steps

Please consider checking for the account's existence before doing call/executecall

[c4-judge]

gzeon-c4 marked the issue as unsatisfactory: Insufficient proof

[c4-sponsor]

livingrockrises marked the issue as sponsor acknowledged