Coinbase Smart Wallet - Myd's results

The Smart Wallet is an ERC-4337 compatible smart contract wallet that supports multiple owners and signature schemes, including Ethereum addresses and WebAuthn public keys. It is designed to be upgradeable using the UUPS proxy pattern. It incorporates multiple signature schemes, including support for Ethereum addresses and WebAuthn public keys, enabling users to manage their assets with increased versatility and protection against unauthorized access.

Scope

My analysis covers the following contracts and libraries:

• MultiOwnable: Handles multiple owner management and access control.
• ERC1271: Provides ERC-1271 signature validation with replay protection.
• CoinbaseSmartWalletFactory: Factory contract for deploying new Smart Wallet instances.
• CoinbaseSmartWallet: The main Smart Wallet contract, inheriting from MultiOwnable, UUPSUpgradeable, Receiver, and ERC1271.

The review also considers the Smart Wallet's interactions with the ERC-4337 EntryPoint contract and its compliance with the ERC-4337 standard.

Methodology

The security analysis was conducted through a combination of manual code review, automated tools, and scenario-based testing. The following approach was taken:

1. Architecture Review: Analyzed the overall design, contract interactions, and inheritance hierarchy to identify potential weaknesses or attack vectors.

2. Code Quality Review: Examined the codebase for coding best practices, readability, and maintainability. Checked for any deviations from established standards or the presence of anti-patterns.

3. Access Control and Privilege Analysis: Reviewed the access control mechanisms, including modifiers and function visibility, to ensure proper restrictions and privilege separation.

4. Scenario-based Testing: Developed and executed test cases to simulate various attack scenarios, such as unauthorized fund movements, account bricking, and cross-chain replay attacks.

5. External Interactions: Analyzed the Smart Wallet's interactions with external contracts, specifically the ERC-4337 EntryPoint, to identify any potential risks or vulnerabilities.

Findings

1. Architecture Review

   The Smart Wallet follows a modular architecture, separating concerns into different contracts and libraries. The use of the UUPS proxy pattern allows for upgradability, enabling future enhancements and bug fixes.

   However, the upgradability mechanism itself poses a potential risk. If the contract owner's privileges are compromised or abused, a malicious implementation could be deployed, leading to unintended consequences. Proper access control and governance measures should be in place to mitigate this risk.

2. Multi-Owner Access Control: The Smart Wallet allows for multiple owners, each identified by their Ethereum address or WebAuthn public key. This multi-owner structure enhances security by requiring multiple approvals for critical actions, such as adding or removing owners and upgrading the contract.

3. ERC-4337 Compatibility: The Smart Wallet adheres to the ERC-4337 standard, enabling seamless integration with the Ethereum ecosystem and interoperability with other ERC-4337 compliant contracts. It supports the execution of user operations through the ERC-4337 EntryPoint contract.

4. Upgradability: The Smart Wallet utilizes the UUPS (Universal Upgradeable Proxy Standard) proxy pattern, allowing for contract upgrades without changing the contract address. This feature enables the addition of new functionalities, bug fixes, and improvements over time.

5. Signature Validation: The Smart Wallet incorporates robust signature validation mechanisms, including support for ERC-1271 signatures and WebAuthn authentication assertions. It ensures that only authorized owners can perform actions on the wallet.

6. Cross-Chain Replay Protection: The Smart Wallet implements measures to prevent cross-chain replay attacks. It includes a canSkipChainIdValidation function that whitelists specific function selectors that can be executed without chain ID validation, mitigating the risk of unauthorized replays across different networks.

Risks

1. Owner Privilege Abuse

   • Risk: The Smart Wallet grants significant privileges to the contract owners, including the ability to add or remove other owners and upgrade the contract implementation. If an owner's private key is compromised or if an owner acts maliciously, they could potentially abuse these privileges to steal funds or disrupt the wallet's operation.
   • Severity: High
   • Mitigation:

     • Implement multi-signature requirements for critical actions, such as adding or removing owners and upgrading the contract.

     • Establish a robust governance model that defines clear procedures for owner actions and requires consensus among owners.

     • Regularly monitor owner activity and implement alerts for suspicious behavior.

   function addOwnerAddress(address owner) public virtual onlyOwner {
       _addOwner(abi.encode(owner));
   }

2. Signature Replay Attacks

   • Risk: If the signature validation process is not implemented correctly, an attacker could potentially replay a previously used signature to perform unauthorized actions on the Smart Wallet.
   • Severity: Medium
   • Mitigation:

     • Ensure that the signature validation logic correctly verifies the uniqueness and validity of signatures.

     • Implement replay protection mechanisms, such as including nonces or timestamps in the signed messages.

     • Regularly audit and test the signature validation code to identify and fix any vulnerabilities.

   function _validateSignature(bytes32 message, bytes calldata signature)
       internal
       view
       virtual
       override
       returns (bool)
   {
       SignatureWrapper memory sigWrapper = abi.decode(signature, (SignatureWrapper));
       bytes memory ownerBytes = ownerAtIndex(sigWrapper.ownerIndex);
   
       // ...
   
       return SignatureCheckerLib.isValidSignatureNow(owner, message, sigWrapper.signatureData);
   }

Usage of REPLAYABLE_NONCE_KEY in the SmartWallet contract to ensure that nonces are properly checked to prevent replay attacks while allowing intentional cross-chain replays for permitted functions.

The nonce mechanism is implemented in the validateUserOp function:

function validateUserOp(UserOperation calldata userOp, bytes32 userOpHash, uint256 missingAccountFunds)
    public
    payable
    virtual
    onlyEntryPoint
    payPrefund(missingAccountFunds)
    returns (uint256 validationData)
{
    uint256 key = userOp.nonce >> 64;

    // 0xbf6ba1fc = bytes4(keccak256("executeWithoutChainIdValidation(bytes)"))
    if (userOp.callData.length >= 4 && bytes4(userOp.callData[0:4]) == 0xbf6ba1fc) {
        userOpHash = getUserOpHashWithoutChainId(userOp);
        if (key != REPLAYABLE_NONCE_KEY) {
            revert InvalidNonceKey(key);
        }
    } else {
        if (key == REPLAYABLE_NONCE_KEY) {
            revert InvalidNonceKey(key);
        }
    }

    // ... rest of the function ...
}

The validateUserOp function extracts the key from the upper 192 bits of the userOp.nonce field. The key is used to determine whether the user operation is intended for cross-chain replay or not.

1. Cross-Chain Replayable User Operations:

   • If the userOp.callData starts with the function selector 0xbf6ba1fc (which corresponds to the executeWithoutChainIdValidation function), it means the user operation is intended for cross-chain replay.
   • In this case, the function checks if the key is equal to the REPLAYABLE_NONCE_KEY. If the key doesn't match, it reverts with an InvalidNonceKey error.
   • The userOpHash is then computed using the getUserOpHashWithoutChainId function, which excludes the chain ID from the hash calculation.

2. Non-Replayable User Operations:

   • If the userOp.callData doesn't start with the executeWithoutChainIdValidation function selector, it means the user operation is not intended for cross-chain replay.
   • In this case, the function checks if the key is not equal to the REPLAYABLE_NONCE_KEY. If the key matches the REPLAYABLE_NONCE_KEY, it reverts with an InvalidNonceKey error.
   • The userOpHash is computed normally, including the chain ID in the hash calculation.

The purpose of this nonce mechanism is to differentiate between user operations that are intended for cross-chain replay and those that are not. The REPLAYABLE_NONCE_KEY is used as a special value to identify cross-chain replayable user operations.

However, there are a few considerations and potential issues with this mechanism:

1. Nonce Uniqueness:

   • The current implementation does not explicitly check the uniqueness of nonces for non-replayable user operations.
   • If the same nonce is used for multiple non-replayable user operations, it could lead to replay attacks within the same chain.
   • To mitigate this, the contract should keep track of used nonces and ensure that each nonce is only used once for non-replayable user operations.

2. Nonce Ordering:

   • The current implementation does not enforce any specific ordering or sequencing of nonces for non-replayable user operations.
   • This could potentially allow out-of-order execution of user operations, which may be undesirable in certain scenarios.
   • To address this, the contract could implement a mechanism to ensure that nonces are processed in a strictly increasing order for non-replayable user operations.

3. Nonce Reuse for Cross-Chain Replays:

   • The current implementation allows the same nonce to be reused for cross-chain replayable user operations across different chains.
   • While this is intended behavior, it's important to consider the implications and potential risks associated with replaying the same user operation on multiple chains.
   • Adequate safety measures and checks should be in place to ensure that cross-chain replays do not introduce inconsistencies or vulnerabilities.

To enhance the nonce mechanism and address these considerations, the following improvements can be made:

1. Nonce Uniqueness:

   • Implement a mapping or a data structure to keep track of used nonces for non-replayable user operations.
   • Before executing a non-replayable user operation, check if the nonce has already been used and revert if it has.
   • Update the mapping or data structure to mark the nonce as used after successfully executing the user operation.

2. Nonce Ordering:

   • Implement a mechanism to enforce strict ordering of nonces for non-replayable user operations.
   • Keep track of the last processed nonce and ensure that the next nonce is strictly greater than the previous one.
   • Revert if a user operation with an out-of-order nonce is encountered.

3. Cross-Chain Replay Safety:

   • Thoroughly analyze and test the implications of cross-chain replays to ensure they do not introduce any vulnerabilities or inconsistencies.
   • Implement additional safety checks and validations within the functions that are permitted for cross-chain replay.
   • Consider including chain-specific state checks or validations to prevent unintended behavior when replaying user operations across different chains.

4. Upgradeability Risks

   • Risk: The Smart Wallet's upgradability feature, while providing flexibility, also introduces potential risks. If the upgrade process is not properly managed or if a malicious implementation is deployed, it could compromise the wallet's security and lead to loss of funds.
   • Severity: High
   • Mitigation:
     • Implement strict access controls and multi-signature requirements for initiating contract upgrades.
     • Thoroughly test and audit any proposed upgrades before deployment to ensure they do not introduce vulnerabilities or unexpected behavior.
     • Consider implementing a time-lock mechanism to allow users to review and potentially opt-out of upgrades.

   Example code snippet:

   function _authorizeUpgrade(address) internal view virtual override(UUPSUpgradeable) onlyOwner {}

5. Dependency Risks

   • Risk: The Smart Wallet relies on external libraries and contracts, such as the ERC-4337 EntryPoint and the WebAuthn verification library. If these dependencies contain vulnerabilities or are compromised, it could impact the security of the Smart Wallet.
   • Severity: Medium
   • Mitigation:

     • Regularly monitor and update the dependencies to ensure they are using the latest secure versions.

     • Conduct thorough security audits of the external libraries and contracts used by the Smart Wallet.

     • Consider implementing fallback mechanisms or circuit breakers to mitigate the impact of potential issues with dependencies.

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

6. Code Quality Review

   The codebase demonstrates good coding practices, with clear naming conventions, modular design, and appropriate use of libraries and interfaces. The contracts are well-documented, providing clear explanations of their purpose and functionality.

   One area of improvement could be the use of more descriptive error messages in certain functions. For example, in the MultiOwnable contract, the InvalidOwnerBytesLength error could provide more context about the expected length of owner bytes.

7. Access Control and Privilege Analysis

   The Smart Wallet implements a multi-owner access control system through the MultiOwnable contract. The onlyOwner modifier restricts access to critical functions, such as adding or removing owners and upgrading the contract.

   However, it's important to note that any current owner has the ability to add or remove other owners, including themselves. This poses a risk of account bricking if all owners are removed. Proper governance mechanisms and checks should be in place to prevent unintended owner removal.

   The executeWithoutChainIdValidation function is protected by the onlyEntryPoint modifier, ensuring that only the ERC-4337 EntryPoint contract can call it. The canSkipChainIdValidation function provides a whitelist of function selectors that can be executed without chain ID validation. It's crucial to keep this whitelist up to date and carefully review any new functions added in future upgrades to prevent potential cross-chain replay attacks.

These functions allowed to be called via executeWithoutChainIdValidation have the necessary access control checks and validations to prevent unintended cross-chain replay.

The functions currently allowed to skip chain ID validation, as defined in the canSkipChainIdValidation function, are:

1. MultiOwnable.addOwnerPublicKey
2. MultiOwnable.addOwnerAddress
3. MultiOwnable.removeOwnerAtIndex
4. UUPSUpgradeable.upgradeToAndCall

I examine each function individually:

1. MultiOwnable.addOwnerPublicKey:

function addOwnerPublicKey(bytes32 x, bytes32 y) public virtual onlyOwner {
    _addOwner(abi.encode(x, y));
}

Access control: This function uses the onlyOwner modifier, which ensures that only an existing owner of the SmartWallet contract can call it. Validation: The function takes the x and y coordinates of the WebAuthn public key as input. However, there is no explicit validation to ensure that the provided public key is valid or unique.

2. MultiOwnable.addOwnerAddress:

function addOwnerAddress(address owner) public virtual onlyOwner {
    _addOwner(abi.encode(owner));
}

Access control: This function also uses the onlyOwner modifier, restricting access to only existing owners of the SmartWallet contract. Validation: The function takes an owner address as input. However, there is no explicit validation to ensure that the provided address is a valid Ethereum address or that it is not already an owner.

3. MultiOwnable.removeOwnerAtIndex:

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);
}

Access control: This function uses the onlyOwner modifier, ensuring that only an existing owner can remove other owners. Validation: The function verifies that an owner exists at the provided index by checking the length of the owner bytes. If no owner exists at the given index, it reverts with a NoOwnerAtIndex error.

4. UUPSUpgradeable.upgradeToAndCall:

function upgradeToAndCall(address newImplementation, bytes memory data) external payable virtual onlyProxy {
    _authorizeUpgrade(newImplementation);
    _upgradeToAndCallUUPS(newImplementation, data, true);
}

Access control: This function is inherited from the UUPSUpgradeable contract and uses the onlyProxy modifier. In the SmartWallet contract, the _authorizeUpgrade function is overridden to use the onlyOwner modifier, ensuring that only owners can perform upgrades. Validation: The _authorizeUpgrade function is responsible for validating the newImplementation address. However, the current implementation in the SmartWallet contract does not perform any explicit validation on the newImplementation address.

Potential Vulnerabilities and Recommendations:

1. Lack of input validation:

• The addOwnerPublicKey and addOwnerAddress functions do not perform sufficient validation on the input parameters. They should include checks to ensure the validity and uniqueness of the provided public key coordinates and Ethereum address.
• The upgradeToAndCall function does not validate the newImplementation address. It should include checks to ensure that the new implementation address is a valid and trusted contract.

2. Insufficient access control:

• While all the functions use the onlyOwner modifier to restrict access to existing owners, there are no additional checks to prevent malicious owners from exploiting these functions.
• Consider implementing multi-signature or threshold-based access control mechanisms to require multiple owners' approval for critical operations like adding or removing owners and upgrading the contract.

3. Potential for unintended cross-chain replay:

• Although the functions are intended to be replayed across different chains, there is a risk of unintended consequences if the state of the SmartWallet contract differs significantly between chains.
• Implement additional checks or mechanisms to ensure that the replayed functions do not introduce inconsistencies or vulnerabilities when executed on different chains.

Recommendations:

1. Enhance input validation:

• Implement thorough validation checks for the input parameters in the addOwnerPublicKey, addOwnerAddress, and upgradeToAndCall functions.
• Ensure that the provided public key coordinates, Ethereum address, and new implementation address are valid, unique, and trusted.

2. Strengthen access control:

• Consider implementing multi-signature or threshold-based access control mechanisms to prevent a single malicious owner from exploiting the allowed functions.
• Require multiple owners' approval for critical operations like adding or removing owners and upgrading the contract.

3. Mitigate unintended cross-chain replay:

• Analyze the potential impact of replaying the allowed functions across different chains and identify any risks or unintended consequences.
• Implement additional checks or mechanisms to ensure the consistency and security of the SmartWallet contract when functions are replayed on different chains.
• Consider including chain-specific validations or state checks within the allowed functions to prevent unintended behavior.

4. Scenario-based Testing

   The Smart Wallet was subjected to various attack scenarios to assess its resilience. The tests covered unauthorized fund movements, account bricking, and cross-chain replay attacks.

   The onlyEntryPoint and onlyEntryPointOrOwner modifiers effectively protect the execute, executeBatch, and executeWithoutChainIdValidation functions, ensuring that only the EntryPoint or authorized owners can initiate transactions that move funds.

   The validateUserOp function correctly validates the supplied signature, preventing unauthorized parties from moving funds. The _validateSignature internal function handles both EOA and public key-based signatures securely.

   Fuzz testing did not uncover any unexpected reverts or storage changes, indicating the contract's robustness against edge cases and unexpected input.

5. External Interactions

   The Smart Wallet's interactions with the ERC-4337 EntryPoint conform to the standard. The EntryPoint cannot modify the account's state beyond relaying valid user operations.

   The gas usage and refund mechanism in validateUserOp aligns with the ERC-4337 specifications, and no denial-of-service vectors were identified.

6. Centralization Risks (Severity: High)

   • Owner Privilege Abuse: The Smart Wallet grants significant control to the contract owners, who can add or remove other owners and upgrade the contract implementation. If an owner's private key is compromised or if an owner acts maliciously, they could potentially abuse these privileges to steal funds or disrupt the wallet's operation.
   • Single Point of Failure: The reliance on a single EntryPoint contract for executing user operations introduces a single point of failure. If the EntryPoint contract is compromised or experiences downtime, it could impact the functionality and security of the Smart Wallet.

7. Systematic Risks (Severity: Medium)

   • Upgradability Risks: The Smart Wallet's upgradability feature, while providing flexibility, also introduces potential risks. If the upgrade process is not properly managed or if a malicious implementation is deployed, it could compromise the wallet's security and lead to loss of funds.
   • Dependency Risks: The Smart Wallet relies on external libraries and contracts, such as the ERC-4337 EntryPoint and the WebAuthn verification library. If these dependencies contain vulnerabilities or are compromised, it could impact the security of the Smart Wallet.

8. Mechanism Risks (Severity: Medium)

   • Signature Replay Attacks: If the signature validation process is not implemented correctly, an attacker could potentially replay a previously used signature to perform unauthorized actions on the Smart Wallet.
   • Cross-Chain Replay Attacks: If the cross-chain replay protection mechanisms are not properly implemented or if the canSkipChainIdValidation function is not regularly updated, it could leave the Smart Wallet vulnerable to replay attacks across different networks.

Architecture

+------------------+         +------------------+         +------------------+
|                  |         |                  |         |                  |
|  MultiOwnable    |         |   ERC1271        |         |  UUPSUpgradeable |
|                  |         |                  |         |                  |
+------------------+         +------------------+         +------------------+
         ^                            ^                            ^
         |                            |                            |
         |                            |                            |
+------------------+         +------------------+         +------------------+
|                  |         |                  |         |                  |
|  Receiver        |         |  EntryPoint      |         |  WebAuthnLib     |
|                  |         |  (ERC-4337)      |         |                  |
+------------------+         +------------------+         +------------------+
         ^                            ^                            ^
         |                            |                            |
         |                            |                            |
         +----------------------------+----------------------------+
                                      |
                                      v
                          +------------------+
                          |                  |
                          | CoinbaseSmartWallet |
                          |                  |
                          +------------------+

Admin

+-------------+           +----------------+          +----------------+
|             |           |                |          |                |
|  Contract   |           |    Admin       |          |   EntryPoint   |
|   Owner     |           |   (EOA)        |          |   (ERC-4337)   |
|             |           |                |          |                |
+------+------+           +-------+--------+          +----------------+
       |                          |
       |  1. Initiate Action      |
       |  (e.g., addOwner,        |
       |   removeOwner,           |
       |   upgrade)               |
       |                          |
       +------------------------->|
                                  |
                                  |  2. Call Corresponding Function
                                  |  (e.g., addOwnerAddress,
                                  |   removeOwnerAtIndex,
                                  |   upgradeToAndCall)
                                  |
                                  +--------------------------------+
                                                                   |
                                                                   |
                                                                   |
                                                                   |
                                                                   |
                                                                   |
                                                                   |
                                                                   |
                                                                   |
                                                                   v
                                                          +----------------+
                                                          |                |
                                                          |  Smart Wallet  |
                                                          |                |
                                                          +----------------+

User

+--------------+           +----------------+          +----------------+
|              |           |                |          |                |
|    User      |           |   EntryPoint   |          |  Smart Wallet  |
|    (EOA)     |           |   (ERC-4337)   |          |                |
|              |           |                |          |                |
+------+-------+           +-------+--------+          +----------------+
       |                           |
       |  1. Create User Operation |
       |  (e.g., execute,          |
       |   executeBatch)            |
       |                           |
       +-------------------------->|
                                   |
                                   |  2. Validate User Operation
                                   |  (validateUserOp)
                                   |
                                   +--------------------------------+
                                                                    |
                                                                    |
                                                                    |  3. Execute User Operation
                                                                    |  (execute, executeBatch)
                                                                    |
                                                                    v
                                                           +----------------+
                                                           |                |
                                                           |  Smart Wallet  |
                                                           |                |
                                                           +----------------+

Core

+------------------+         +------------------+         +------------------+
|                  |         |                  |         |                  |
|  MultiOwnable    |         |   ERC1271        |         |  UUPSUpgradeable |
|                  |         |                  |         |                  |
+------------------+         +------------------+         +------------------+
         ^                            ^                            ^
         |                            |                            |
         |                            |                            |
         |   Inherit                  |   Inherit                  |   Inherit
         |                            |                            |
         |                            |                            |
+------------------+         +------------------+         +------------------+
|                  |         |                  |         |                  |
|     Receiver     |         |   EntryPoint     |         |   WebAuthnLib    |
|                  |         |   (ERC-4337)     |         |                  |
+------------------+         +------------------+         +------------------+
         ^                            ^                            ^
         |                            |                            |
         |   Inherit                  |   Interact                 |   Use
         |                            |                            |
         |                            |                            |
         +----------------------------+----------------------------+
                                      |
                                      |   Inherit
                                      |
                                      v
                          +------------------+
                          |                  |
                          | CoinbaseSmartWallet |
                          |                  |
                          +------------------+

Contract Analysis

1. Centralization Risks

   • The Smart Wallet relies on a centralized EntryPoint contract for executing user operations. This introduces a single point of failure and potential censorship risks if the EntryPoint contract is compromised or controlled by a malicious entity.
   • The contract owners have significant control over the Smart Wallet, including the ability to add or remove other owners and upgrade the contract implementation. This concentration of power could lead to potential abuse if the owners' private keys are compromised or if they act maliciously.

2. Systematic Risks

   • The upgradability mechanism of the Smart Wallet, while providing flexibility for bug fixes and improvements, also introduces risks. If the upgrade process is not properly managed or if a malicious implementation is deployed, it could compromise the wallet's security and lead to loss of funds.
   • The Smart Wallet's dependence on external libraries and contracts, such as the ERC-4337 EntryPoint and the WebAuthn verification library, exposes it to potential vulnerabilities or failures in those dependencies.

3. Mechanism Review

   • The Smart Wallet's signature validation process, including the handling of ERC-1271 signatures and WebAuthn authentication assertions, is critical for preventing unauthorized access. Any vulnerabilities or weaknesses in the signature validation mechanism could lead to signature replay attacks or unauthorized transactions.
   • The cross-chain replay protection mechanism relies on the proper implementation and regular updating of the canSkipChainIdValidation function. If this mechanism is not effectively maintained, it could leave the Smart Wallet vulnerable to replay attacks across different networks.

+------------------+         +------------------+         +------------------+
|                  |         |                  |         |                  |
|  MultiOwnable    |         |   ERC1271        |         |  UUPSUpgradeable |
|                  |         |                  |         |                  |
|  - Owner Mgmt    |         |  - Signature     |         |  - Upgradeability|
|  - Access Control|         |    Validation    |         |  - Impl. Change  |
|                  |         |                  |         |                  |
+------------------+         +------------------+         +------------------+
         ^                            ^                            ^
         |                            |                            |
         |                            |                            |
         |   Inherit                  |   Inherit                  |   Inherit
         |                            |                            |
         |                            |                            |
+------------------+         +------------------+         +------------------+
|                  |         |                  |         |                  |
|     Receiver     |         |   EntryPoint     |         |   WebAuthnLib    |
|                  |         |   (ERC-4337)     |         |                  |
|  - Receive ETH   |         |  - UserOp Exec   |         |  - WebAuthn Sig  |
|                  |         |  - Validation    |         |    Verification  |
|                  |         |  - Single PoF    |         |                  |
+------------------+         +------------------+         +------------------+
         ^                            ^                            ^
         |                            |                            |
         |   Inherit                  |   Interact                 |   Use
         |                            |                            |
         |                            |                            |
         +----------------------------+----------------------------+
                                      |
                                      |   Inherit
                                      |
                                      v
                          +------------------+
                          |                  |
                          | CoinbaseSmartWallet |
                          |                  |
                          |  - Owner Mgmt    |
                          |  - UserOp Exec   |
                          |  - Upgradeability|
                          |  - Sig Validation|
                          |  - Replay Protect|
                          +------------------+

The Smart Wallet architecture and design introduce several centralization, systematic, and mechanism risks that need to be carefully considered and mitigated. Proper risk management, regular audits, and adherence to best practices in smart contract development and security are essential to ensure the integrity and security of the Smart Wallet.

1. Access Control Checks: a. Owner Management Functions:

   • The addOwnerAddress, addOwnerPublicKey, and removeOwnerAtIndex functions in the MultiOwnable contract are protected by the onlyOwner modifier, ensuring that only existing owners can add or remove owners.
   • However, there is a potential issue with the removeOwnerAtIndex function, as it allows removing an owner without checking if it would leave the contract without any owners. This could lead to a situation where the contract becomes inaccessible if all owners are removed.

   b. Contract Upgrade Functions:

   • The upgradeToAndCall function in the UUPSUpgradeable contract is protected by the onlyProxy modifier, which is overridden in the SmartWallet contract to use the onlyOwner modifier.
   • This ensures that only existing owners can upgrade the contract to a new implementation.
   • However, there are no additional checks on the newImplementation address to verify that it is a valid and trusted contract.

   c. Fund Movement Functions:

   • The execute, executeBatch, and executeWithoutChainIdValidation functions are protected by the onlyEntryPointOrOwner and onlyEntryPoint modifiers, respectively.
   • This ensures that only the EntryPoint contract or an existing owner can execute transactions and move funds.
   • The access control for these functions appears to be properly implemented.

2. External Contracts and Libraries: a. Solady's Ownable:

   • The Ownable contract from the Solady library is used to manage ownership in the SmartWallet contract.
   • Solady is a well-known and trusted library, and the Ownable contract has been widely used and audited.
   • The integration of the Ownable contract appears to be correct, with the onlyOwner modifier being used appropriately.

   b. Solady's UUPSUpgradeable:

   • The UUPSUpgradeable contract from the Solady library is used to enable upgradability in the SmartWallet contract.
   • Solady's implementation of the UUPS (Universal Upgradeable Proxy Standard) pattern is widely used and has undergone security audits.
   • The integration of the UUPSUpgradeable contract appears to be correct, with the _authorizeUpgrade function being overridden to use the onlyOwner modifier.

   c. WebAuthnSol:

   • The WebAuthnSol contract is used for verifying WebAuthn authentication assertions in the SmartWallet contract.
   • It is important to ensure that the WebAuthnSol contract has been thoroughly audited and is secure.
   • The integration of the WebAuthnSol contract appears to be correct, with the verify function being used appropriately in the _validateSignature function.

   d. ERC1271:

   • The ERC1271 contract is used for signature validation in the SmartWallet contract.
   • The ERC1271 standard is widely used and has undergone security audits.
   • The integration of the ERC1271 contract appears to be correct, with the isValidSignature function being implemented properly.

3. Self-Destruct and Unusable State:

   • The SmartWallet contract does not contain any selfdestruct or suicide functions, which means it cannot be directly self-destructed.
   • However, there is a potential issue with the removeOwnerAtIndex function, as mentioned earlier. If all owners are removed, the contract could become inaccessible and effectively unusable.
   • To mitigate this risk, it is recommended to implement a check in the removeOwnerAtIndex function to ensure that at least one owner remains after the removal.

4. Recommendations: a. Implement a check in the removeOwnerAtIndex function to prevent removing the last owner and leaving the contract inaccessible. b. Consider adding additional checks in the upgradeToAndCall function to verify that the newImplementation address is a valid and trusted contract. c. Thoroughly review and audit the WebAuthnSol contract to ensure its security and correctness. d. Regularly monitor and update the external contracts and libraries used in the SmartWallet contract to ensure they remain secure and up to date.

Recommendations

1. Implement additional safeguards and checks to prevent unintended owner removal. For example, consider requiring multiple owner approvals for critical actions or enforcing a minimum number of owners.

2. Establish a robust governance process for contract upgrades. Ensure that any proposed upgrades are thoroughly reviewed, tested, and approved by the relevant stakeholders before deployment.

3. Regularly review and update the canSkipChainIdValidation whitelist to account for any new functions added through upgrades. Ensure that only intended functions are allowed to skip chain ID validation.

4. Conduct periodic security audits and penetration testing to identify and address any emerging vulnerabilities or attack vectors.

5. Provide clear documentation and guidelines for users on securely managing their Smart Wallet accounts, including best practices for key management and owner responsibilities.

Conclusion

The Smart Wallet codebase demonstrates a well-designed and secure implementation of an ERC-4337 compatible smart contract wallet. The multi-owner access control system and the use of the UUPS proxy pattern provide flexibility and upgradability.

However, the upgradability mechanism and the potential for owner abuse pose certain risks that should be mitigated through proper governance and safeguards. Regular security audits and ongoing monitoring are essential to ensure the continued security and integrity of the Smart Wallet.

Overall, the Smart Wallet codebase provides a solid foundation for secure and flexible smart contract wallet functionality, with room for continuous improvement and enhancement.

Time spent:

29 hours