Coinbase Smart Wallet - cheatc0d3's results

Lines of code

https://github.com/code-423n4/2024-03-coinbase/blob/main/src/MagicSpend/MagicSpend.sol#L109

Vulnerability details

Impact

A malicious user could potentially exploit certain conditions to cause functions like validatePaymasterUserOp to revert consistently. If these reverts happen frequently, it could lead to the paymaster being flagged, deprioritized by the EntryPoint, or even banned due to perceived unreliability.

Proof of Concept

A bad actor could say observe a pending transaction in the bundler mempool that will significantly reduce the paymaster's balance, for instance a large withdrawal by a legitimate user. Before this transaction is mined, the frontrunner executes another transaction that further depletes the contract's balance. This action could make the original transaction fail due to insufficient funds.

Github link

if (address(this).balance < withdrawAmount) {
            revert InsufficientBalance(withdrawAmount, address(this).balance);
        }
        

The EntryPoint relies on validatePaymasterUserOp to not revert under normal operation. Consistent reverts, especially due to balance issues caused by frontrunning, signal to the EntryPoint that the paymaster is unreliable.

Scenario

Front-runners monitor the mempool for transactions indicating significant balance changes or withdrawals from the paymaster. They then submit their transactions with higher gas fees which leads to their transactions being mined before the observed transactions.

This frontrunning can cause legitimate transactions to fail due to changed state conditions like insufficient balance, which in turn could cause validatePaymasterUserOp to revert consistently. Consequently signaling to Entrypoint that the paymaster is unreliable leading to it been banned or unprioritized for future transactions in the network.

Tools Used

Manual Review

Recommended Mitigation Steps

1. Use private transaction relayers or services like Flashbots to prevent frontrunners from observing and exploiting transactions in the public mempool.

2. Maintain a buffer of funds that are not immediately visible or accessible through withdrawal requests. This buffer can help ensure that observed transactions in the mempool do not lead to state changes that cause subsequent transactions to fail.

Assessed type

DoS

QA Report: Smart Wallet

L-01 Employ ERC-1167 Clones for smart wallets

Using ERC-1167 clones allows each deployed smart wallet to operate independently (having its own state) while still using the same underlying logic. This is ideal for user-specific wallet contracts where each user gets a separate wallet but all wallets share the same functionality.

The primary feature of ERC-1167, creating lightweight proxy contracts, aligns with the need to deploy multiple smart wallet instances without the high cost associated with deploying full contract code for each instance. This is particularly beneficial for a factory pattern that aims to produce many instances of a particular contract type.

L-02 Integration Constraints Imposed by isContract Checks

Many DeFi protocols and other smart contract systems use isContract checks to determine if a caller is an EOA or another smart contract. This is often used for security reasons or to enforce certain behaviors. For example, a contract might restrict certain functions to only be callable by EOAs to prevent reentrancy attacks or other contract-based exploits.

Interoperability Issues: If a project uses isContract checks to restrict interactions to EOAs, then smart contract wallets deployed as minimal proxies (or any smart contract, for that matter) would be unable to interact with these functions directly. This could limit the usability of smart contract wallets with such protocols, requiring additional steps or alternative approaches for users of smart contract wallets to engage with these systems.

L-03 Denial of Service Due to Payload Size

Large initCode, callData, and paymasterAndData payloads can increase transaction costs and potentially exceed block gas limits, affecting the network and contract's performance.

Loc:

Impose size limits on initCode, callData, and paymasterAndData to prevent excessive transaction sizes.

require(userOp.initCode.length <= MAX_INIT_CODE_SIZE, "initCode too large");
require(userOp.callData.length <= MAX_CALL_DATA_SIZE, "callData too large");

L-04 Denial of Service Due to High Gas Limits

callGasLimit, verificationGasLimit, preVerificationGas, maxFeePerGas, and maxPriorityFeePerGas could be manipulated to attempt DoS attacks by specifying gas limits that are too high or too low, potentially leading to transaction failure or exploiting the paymaster's funds.

Loc:

Set maximum limits for callGasLimit and verificationGasLimit based on typical operation costs.

require(userOp.callGasLimit <= MAX_CALL_GAS_LIMIT, "Excessive call gas limit");
require(userOp.verificationGasLimit <= MAX_VERIFICATION_GAS_LIMIT, "Excessive verification gas limit");

L-05 Enforce Sequential Nonces

Ensure nonces are used sequentially without gaps, effectively mitigating replay attacks. This would involve tracking the last used nonce for each user and ensuring each new operation increments this nonce.

uint256 expectedNonce = lastNonce[userOp.sender] + 1;
require(userOp.nonce == expectedNonce, "Invalid nonce");
lastNonce[userOp.sender] = expectedNonce;

L-06 Limit Operation Frequency

For functions that involve stake management like entryPointAddStake, entryPointUnlockStake, and entryPointWithdrawStake impose a rate limit.

This reduces the risk of destabilizing the contract's operations through rapid, successive changes, allowing for more stable management of resources.

L-07 No Setters for Configurable Parameters

Critical parameters like the EntryPoint address are hard-coded or lack the flexibility for updates. In a dynamic blockchain environment, the ability to update certain parameters without redeploying the contract could be crucial.

Add this:

Function: setEntryPointAddress(address newEntryPoint) Purpose: Allows the admin to update the EntryPoint address. Impact: Enhances flexibility and adaptability to changes in the underlying account abstraction infrastructure.

L-08 Lack of Emergency Stop Mechanism

In case of detected vulnerabilities or attacks, an emergency stop mechanism (often implemented as a "circuit breaker") can temporarily halt critical contract operations.

Add pause and unpause functions. This will increases security by allowing immediate response to emergencies, protecting funds and preventing potential misuse.

L-09 Lack of Withdrawal Limits

To further safeguard against unauthorized or accidental large withdrawals, setting daily or transactional limits could be beneficial.

Cansider adding this function, setWithdrawalLimit(uint256 newLimit)

It sets the maximum amount that can be withdrawn within a certain timeframe.

This prevents excessive withdrawals, adding an additional layer of financial security.

L-10 No Role-Based Access Control

Allows for differentiated access levels, enabling more secure and flexible administration of the contract.

Improves security by limiting critical operations to authorized roles and facilitates delegation of specific responsibilities.

L-11 Lack of Recovery Options for Lost Access

In case the owner's private key is lost or compromised, having a recovery mechanism can be crucial to regain control over the contract.

Add these functions: -initiateRecovery(address newOwner, uint256 recoveryDelay) -finalizeRecovery()

This starts and completes the process of transferring ownership after a security delay.

It adds resilience to the contract by providing a path to recover from lost access or compromised keys, without compromising immediate security.

L-12 No whitelists of supported tokens

Although the current implementation only supports withdrawing ETH (identified by the zero address), since you are planning to support other assets in the future. Implementing a check against a list of supported assets would make it easier to extend

mapping(address => bool) public supportedAssets;

// In the constructor or a separate function
supportedAssets[address(0)] = true; // ETH

function _validateAsset(address asset) internal view {
    if (!supportedAssets[asset]) revert UnsupportedPaymasterAsset(asset);
}

L-13 Emitting Misleading Events via OwnerWithdraw

The ownerWithdraw function allows the contract owner to withdraw assets to any address. If the contract owner acts maliciously or the owner's account is compromised, this function could be used to emit MagicSpendWithdrawal events that do not represent genuine user withdrawals.

Misuse of this functionality could flood the event log with false withdrawals, complicating the tracking of legitimate user activity and potentially misleading off-chain analytics or auditors.-

Implementing a multi-signature requirement for sensitive owner operations can mitigate this risk.

L-14 Inflated Withdrawal Events

A compromised admin account could repeatedly invoke functions that emit events, such as making numerous small withdrawals if the contract logic permits. This could lead to an inflated number of events, overwhelming off-chain analytics tools, and making it difficult to discern legitimate activities.

Introducing rate limits or thresholds for operations that trigger events can prevent abuse. For instance, setting a minimum withdrawal amount could deter the generation of numerous minimal withdrawal events.

NC-01 Use of Magic Numbers

To improve clarity the number 160 could be defined as a named constant that explains its purpose.

// Represents the bit length of an Ethereum address, used for encoding purposes.
uint256 public constant ADDRESS_BIT_LENGTH = 160;

Then, you could use ADDRESS_BIT_LENGTH instead of the hard-coded 160 in your bit manipulation, making the intention behind this operation clearer to readers of the code:

validationData = (sigFailed ? 1 : 0) | (uint256(withdrawRequest.expiry) << ADDRESS_BIT_LENGTH);

NC-02 Hardcoded Addresses

Replace hardcoded addresses with named constants for clarity and potentially reducing gas costs if these constants are used multiple times.

address public constant ENTRY_POINT_ADDRESS = 0x5FF137D4b0FDCD49DcA30c7CF57E578a026d2789;
address public constant ETH_ADDRESS = address(0);

Coinbase Smart Wallet Security Analysis Report

Executive Summary

During my audit of the Coinbase Smart Wallet, I identified various security concerns and operational inefficiencies. This report outlines my findings and provides actionable steps for mitigation. The issues range from critical vulnerabilities that could lead to financial losses or contract malfunctions, to optimizations that can significantly reduce gas costs and improve contract efficiency. It's crucial for the development team to prioritize these findings to enhance the security, reliability, and performance of the smart wallet.

---

Scope

The scope of this audit focused on key components of the Coinbase Smart Wallet system, including the SmartWallet contract, WebAuthnSol, FreshCryptoLib, and the MagicSpend module. The aim was to identify security vulnerabilities, architectural weaknesses, and any deviations from best practices that could impact the system's integrity, security, and functionality. Specifically, the audit concentrated on areas susceptible to cross-chain replay attacks, adherence to validation protocols, the integrity of cryptographic operations, and the efficiency and security of the paymaster functions within the MagicSpend module. This comprehensive approach ensured that critical aspects of the Coinbase Smart Wallet were examined to safeguard against potential exploits and operational inefficiencies.

---

Architecture Overview

SmartWallet

The SmartWallet is designed to support cross-chain operations but relies on executeWithoutChainIdValidation for cross-chain replay, which assumes gas values from one chain will be valid on another. This assumption poses risks due to variable gas requirements across different blockchains.

WebAuthnSol

The WebAuthn.sol contract focuses on authentication, explicitly outlining certain validation steps that are intentionally skipped. This approach raises concerns regarding the robustness of authentication mechanisms implemented within the system.

FreshCryptoLib

FreshCryptoLib is a library aimed at providing cryptographic functions, with the understanding that exploits are primarily relevant when initiated through ecdsa_verify. The audit identified specific issues in ecAff_isOnCurve and ecZZ_mulmuladd_S_asm functions, which have been addressed post-discovery in a previous audit. These fixes were crucial for ensuring the cryptographic integrity of operations within the library.

MagicSpend

Operating as a paymaster, MagicSpend interfaces with the EntryPoint contract, managing gas payments for transactions. Issues were found regarding how MagicSpend calculates and covers gas costs, potentially leading to discrepancies in balance management. Additionally, the use of address.balance in validatePaymasterUserOp was identified as a violation of ERC-7562 rules, although a pending PR aims to rectify this.

The architecture of the Coinbase Smart Wallet is multifaceted, with each component playing a critical role in the system's overall functionality and security. The identified issues and the subsequent fixes are steps towards enhancing the robustness and reliability of the system, addressing both security vulnerabilities and architectural inefficiencies.

Findings

Vulnerability to Front-running Attacks on Bundled Transactions

I discovered that the smart wallet is susceptible to front-running attacks, specifically targeting bundled transactions. Malicious actors could observe and exploit transactions waiting in the mempool, leading to financial losses or even causing the paymaster to be banned due to perceived unreliability. This vulnerability arises from a lack of safeguards against sudden balance decreases that might cause transactions to fail.

• Implement private transaction relayers or services like Flashbots to obscure transactions from the public mempool.

• Maintain a buffer of funds that aren't immediately available for withdrawal, providing a safeguard against unexpected balance drops.

---

Advantages of Using ERC-1167 Clones for Smart Wallets

I found that employing ERC-1167 clones for smart wallets can significantly reduce deployment costs and enhance operational efficiency. This method allows for each wallet instance to operate independently while sharing the same underlying logic, ideal for creating multiple user-specific wallets efficiently.

Integrate ERC-1167 clone contracts into the smart wallet deployment process to leverage their cost and operational efficiencies.

---

Integration Constraints Due to isContract Checks

The smart wallet's integration with certain DeFi protocols and smart contract systems might be limited due to isContract checks. These checks can restrict the wallet's ability to interact with contracts requiring caller differentiation, potentially limiting its functionality and interoperability.

Explore alternative methods or workarounds for interaction with protocols employing isContract checks to ensure broad compatibility and functionality.

---

Denial of Service Risks Due to Excessive Payload Size

I identified a potential Denial of Service (DoS) risk stemming from the allowance of large initCode, callData, and paymasterAndData payloads. These can increase transaction costs and risk exceeding block gas limits, impacting the network and the contract's performance.

Implement strict size limits on initCode, callData, and paymasterAndData to mitigate the risk of excessive transaction sizes.

---

Risks of High Gas Limits

The smart wallet's settings allow for potentially dangerous gas limit specifications, which could be exploited to perform DoS attacks or manipulate transaction outcomes. High or low gas limits could lead to transaction failure or the exploitation of the paymaster's funds.

Define and enforce maximum gas limits for callGasLimit and verificationGasLimit based on typical operation costs to prevent abuse.

---

Optimizing Storage Operations

I noted that the smart wallet could achieve gas savings by consolidating storage operations. Currently, multiple updates to a user's withdrawable ETH balance occur in separate transactions, leading to unnecessary gas consumption.

Consolidate changes to a user's balance into a single storage operation to minimize gas usage.

---

Conclusion

This report highlights critical and optimization-level issues discovered during my audit of the Coinbase Smart Wallet. Addressing these findings will not only enhance the security and operational efficiency of the smart wallet but also ensure its long-term sustainability and user trust. It's recommended that the team prioritizes these issues based on their impact and implements the suggested actionable steps to mitigate the identified risks.

Time Spent 36 hrs

Time spent:

36 hours