Coinbase Smart Wallet - 7ashraf's results

Quality Assurance Report

Summary

[L-01] Check if msg.value is the same as the passed amount

Instances

• MagicSpend.sol#212

    function entryPointDeposit(uint256 amount) external payable onlyOwner {
        SafeTransferLib.safeTransferETH(entryPoint(), amount);
    }

• CoinbaseSmartWallet.sol#273

        (bool success, bytes memory result) = target.call{value: value}(data);

[L-02] Avoid hardcoded strings and addresses

Using hardcoded addresses or strings should be avoidable and may overcomplicate things in case of a contract upgrade, rather store contract addresses in a variable and add a setter to change the address in case of an upgrade

Instances

• MagicSpend.sol#304

    function entryPoint() public pure returns (address) {
        return 0x5FF137D4b0FDCD49DcA30c7CF57E578a026d2789;
    }

• CoinbaseSmartWallet.sol#218

    function entryPoint() public view virtual returns (address) {
        return 0x5FF137D4b0FDCD49DcA30c7CF57E578a026d2789;
    }

• CoinbaseSmartWallet.sol#334

        return ("Coinbase Smart Wallet", "1");

[L-04] More safety is advised when removing owners

Instances

• MultiOwnable.sol#102

    function removeOwnerAtIndex(uint256 index) public virtual onlyOwner {
        bytes memory owner = ownerAtIndex(index);
        if (owner.length == 0) revert NoOwnerAtIndex(index);

        delete _getMultiOwnableStorage().isOwner[owner];
        delete _getMultiOwnableStorage().ownerAtIndex[index];

        emit RemoveOwner(index, owner);
    }

Mitigation

• It is recommended to not allow the user to remove himself
• It is recommended to check if the system has at least only one owner to avoid the removal of all owners

[N-01] Remove unnecessary checks

Instances

• MagicSpend.sol#150

        assert(mode != PostOpMode.postOpReverted);

[N-02] Consider emitting an event for the following functions

Instances

• MagicSpend.sol#169

function withdrawGasExcess() external

• MagicSpend.sol#203

function ownerWithdraw(address asset, address to, uint256 amount) external onlyOwner

[N-03] Add comments to explain assembly code

Instances

• CoinbaseSmartWallet.sol#94

        assembly ("memory-safe") {
            if missingAccountFunds {
                // Ignore failure (it's EntryPoint's job to verify, not the account's).
                pop(call(gas(), caller(), missingAccountFunds, codesize(), 0x00, codesize(), 0x00))
            }
        }

• CoinbaseSmartWallet.sol#275

            assembly ("memory-safe") {
                revert(add(result, 32), mload(result))
            }

• CoinbaseSmartWallet.sol#309

           assembly ("memory-safe") {
                owner := mload(add(ownerBytes, 32))
            }

[N-04] Inaccurate variable emission

Instances

• CoinbaseSmartWallet.sol#324

        revert InvalidOwnerBytesLength(ownerBytes);

Mitigation

Change to ownerBytes.length