wilsoncusack/2024-03-coinbase

Smart Wallet audit details

• Total Prize Pool: $49,000 in USDC

  • HM awards: $24,750 in USDC
  • QA awards: $750 in USDC
  • Bot Race awards: $2,250 in USDC
  • Analysis awards: $1,500 in USDC
  • Gas awards: $750 in USDC
  • Judge awards: $5,600 in USDC
  • Lookout awards: $2,400 in USDC
  • Scout awards: $500 in USDC
  • Mitigation Review: $10,500 in USDC (Opportunity goes to top 3 certified wardens based on placement in this audit.)

• Join C4 Discord to register

• Submit findings using the C4 form

• Read our guidelines for more details

• Starts March 14, 2024 20:00 UTC

• Ends March 21, 2024 20:00 UTC

Automated Findings / Publicly Known Issues

The 4naly3er report can be found here.

Automated findings output for the audit can be found here within 24 hours of audit opening.

Note for C4 wardens: Anything included in this Automated Findings / Publicly Known Issues section is considered a publicly known issue and is ineligible for awards.

• SmartWallet
  • Crosschain replay via executeWithoutChainIdValidation relies on gas values from one chain being valid and accepted by a bundler on other chains. This may not be the case.
• WebAuthnSol
  • Read comments on WebAuthn.sol for details on validation steps we are knowingly skipping
• FreshCryptoLib
  • Exploits should only be considered in the context of a call path starting with ecdsa_verify. Other functions are not intended to be called directly.
• MagicSpend
  • When acting as a paymaster, EntryPoint will debit MagicSpend slightly more than actualGasCost, meaning what is withheld on a gas-paying withdraw will not cover 100% of MagicSpend's balance decrease in the EntryPoint.
  • validatePaymasterUserOp checks address.balance, which currently violates ERC-7562 rules, however there is PR to change this.

Overview

This audit covers four separate but related groups of code

• SmartWallet is a smart contract wallet. In addition to Ethereum address owners, it supports passkey owners and validates their signatures via WebAuthnSol. It supports multiple owners and allows for signing account-changing user operations such that they can be replayed across any EVM chain where the account has the same address. It is ERC-4337 compliant and can be used with paymasters such as MagicSpend.
• WebAuthnSol is a library for verifying WebAuthn Authentication Assertions onchain.
• FreshCryptoLib is an excerpt from FreshCryptoLib, including the function ecdsa_verify and all code this function depends on. ecdsa_verify is used by WebAuthnSol onchains without the RIP-7212 verifier, and FCL.n is used to check for signature malleability.
• MagicSpend is a contract that allows for signature-based withdraws. MagicSpend is a EntryPoint v0.6 compliant paymaster and also allows using withdraws to pay transaction gas, in this way.

Links

• Previous audits:
  • SmartWallet
    • Cantina
  • WebAuthnSol
    • Cantina
    • Certora (link pending)
  • FreshCryptoLib
    • Coinbase completed an audit of FreshCryptoLib with artifacts here.
      • Audit doc
      • Testing methodology
  • MagicSpend
    • Cantina
• Documentation: Each folder has a detailed README. Please read those.
• Demo!: You can try using all of these contracts together on our demo site.
• Explainer Video: Here's a video talking through the demo and what is going on behind the scenes.

Scope

Contract	SLOC	Purpose	External Imports
src/SmartWallet/MultiOwnable.sol	80	Auth contract, supporting multiple owners and owners identified as bytes to allow for secp256r1 public keys
src/SmartWallet/ERC1271.sol	54	Abstract contract for ERC-1271 support for CoinbaseSmartWallet
src/SmartWallet/CoinbaseSmartWalletFactory.sol	35	ERC-4337 compliant Factory for CoinbaseSmartWallet	solady/utils/LibClone.sol
src/SmartWallet/CoinbaseSmartWallet.sol	165	ERC-4337 compliant smart account	solady/accounts/Receiver.sol solady/utils/UUPSUpgradeable.sol solady/utils/SignatureCheckerLib.sol account-abstraction/interfaces/UserOperation.sol
src/WebAuthnSol/WebAuthn.sol	54	Solidity WebAuthn verifier	solady/utils/LibString.sol openzeppelin-contracts/contracts/utils/Base64
src/FreshCryptoLib/FCL.sol	255	Library for verifying secp256r1 signatures
src/MagicSpend/MagicSpend.sol	143	Contract supporting signature-based withdraws. Also ERC-4337 EntryPoint v0.6 compliant paymaster	solady/auth/Ownable.sol solady/utils/SignatureCheckerLib.sol solady/utils/SafeTransferLib.sol account-abstraction/interfaces/UserOperation.sol account-abstraction/interfaces/IPaymaster.sol account-abstraction/interfaces/IEntryPoint.sol

Out of scope

The complete scope of this audit is the files included in src/

Additional Context

• Which blockchains will this code be deployed to, and are considered in scope for this audit?

  • We have near-term plans to deploy this code to the mainnets of the following chains: Ethereum, Base, Optimism, Arbitrum, Polygon, BNB, Avalanche, Gnosis.

• Roles/Permissions

  • SmartWallet
    • Only owner or self
      • MultiOwnable.addOwnerAddress
      • MultiOwnable.addOwnerPublicKey
      • MultiOwnable.AddOwnerAddressAtIndex
      • MultiOwnable.addOwnerPublicKeyAtIndex
      • MultiOwnable.removeOwnerAtIndex
      • UUPSUpgradable.upgradeToAndCall
    • Only EntryPoint, owner, or self
      • CoinbaseSmartWallet.execute
      • CoinbaseSmartWallet.executeBatch
    • Only EntryPoint
      • CoinbaseSmartWallet.executeWithoutChainIdValidation
      • validateUserOp
  • MagicSpend
    • Only owner
      • ownerWithdraw
      • entryPointDeposit
      • entryPointWithdraw
      • entryPointAddStake
      • entryPointUnlockStake
      • entryPointWithdrawStake

• ERC/EIP Compliance

  • ERC1271: Should comply with ERC1271
  • CoinbaseSmartWalletFactory: Should comply with factory behavior defined in ERC4337
  • CoinbaseSmartWallet: Should comply with account behavior defined in ERC4337
  • MagicSpend: Should comply with paymaster behavior defined in ERC4337

Attack ideas (Where to look for bugs)

• SmartWallet
  • Can an attacker move funds from the account?
  • Can an attacker brick (make unusable) the account?
  • Can functions not in canSkipChainIdValidation be used via executeWithoutChainIdValidation?
• MagicSpend
  • Can an attacker withdraw using an invalid WithdrawRequest?
  • Can an attacker be credited more than WithdrawRequest.amount?
  • Are there any griefing attacks that could cause this paymaster to be banned by bundlers?
• WebAuthn
  • False positive or false negative in validation
    • Are there valid webauthn authentication assertions that do not pass our validation?
• FreshCryptoLib
  • False positive or false negative in validation

Main invariants

• SmartWallet
  • Only current owners or EntryPoint can make calls that
    • Decrease account balance.
    • Add or remove owner.
    • Upgrade account.
  • Any current owner can add or remove any other owner.
• MagicSpend
  • Only owner can
    • Move funds from contract without a valid WithdrawRequest.
    • Stake and unstake in EntryPoint.
    • Add and withdraw from EntryPoint balance.
  • Every WithdrawRequest can only be used once.
  • A WithdrawRequest cannot be used past WithdrawRequest.expiry.
  • Withdrawers can never receive more than WithdrawRequest.amount.
  • Withdrawers using paymaster functionality should receive exactly WithdrawRequest.amount - postOp_actualGasCost.
  • At the end of a transaction, _withdrawableETH contains no non-zero balances.
• WebAuthn
  • Validation passes if and only if
    • '"challenge":""<challenge>" occurs in clientDataJSON starting at challengeIndex.
    • '"type":"webauth.get" is occurs in clientDataJSON starting at typeIndex.
    • User presence bit is set.
    • User verified bit is set, if required.
    • r and s are valid signature values for x, y on the message hash that results from clientDataJSON and authenticatorData.
• FreshCryptoLib
  • All calls with valid sets of message, r, s, Qx, and Qy for the secp256r1 curve should return true.
  • All calls with invalid sets of message, r, s, Qx, and Qy for the secp256r1 curve should revert or return false.

Scoping Details

- If you have a public code repo, please share it here: https://github.com/coinbase/smart-wallet, https://github.com/coinbase/magic-spend, https://github.com/base-org/webauthn-sol, https://github.com/base-org/fresh-crypto-lib-audit  
- How many contracts are in scope?: 7   
- Total SLoC for these contracts?: 786  
- How many external imports are there?: 11  
- How many separate interfaces and struct definitions are there for the contracts within scope?: 0  
- Does most of your code generally use composition or inheritance?: Composition   
- How many external calls?: 5   
- What is the overall line coverage percentage provided by your tests?: 95
- Is this an upgrade of an existing system?: False
- Check all that apply (e.g. timelock, NFT, AMM, ERC20, rollups, etc.): 
- Is there a need to understand a separate part of the codebase / get context in order to audit this part of the protocol?: False  
- Please describe required context:   
- Does it use an oracle?: No
- Describe any novel or unique curve logic or mathematical models your code uses: 
- Is this either a fork of or an alternate implementation of another project?: True   
- Does it use a side-chain?:
- Describe any specific areas you would like addressed:

Tests

This repository is managed using Foundry.

Install Foundry

Run the following command and then follow the instructions.

curl -L https://foundry.paradigm.xyz | bash

Install Modules

forge install

Run Tests

forge test

Solidity 0.8.23 is used to compile and test the smart contracts.

Miscellaneous

Employees of Coinbase and employees' family members are ineligible to participate in this audit.