Shell Protocol - albahaca's results

Overview of protocol:

The provided codebase consists of multiple Solidity smart contracts designed for interacting with Curve Finance pools, specifically the 2pool and Tricrypto pool, through the Ocean platform. Additionally, there is an abstract contract, OceanAdapter, intended to be extended by other contracts for interacting with Ocean and external primitives. The Ocean contract serves as a central hub for token wrapping, unwrapping, and various interactions with ERC-20, ERC-721, and ERC-1155 tokens.

Any comments for the judge:

The codebase appears well-structured and follows modern Solidity practices. However, it poses potential security risks, especially in the unlimited token approvals, reliance on external contracts, and assumptions about the correctness of external addresses.

Approach taken in evaluating the codebase:

The evaluation involved a thorough analysis of each contract's structure, immutables, state variables, functions, and potential security considerations. Attention was given to token approvals, external contract interactions, reentrancy risks, and overall adherence to best practices.

Architecture recommendations:

a. Limited Token Approvals: Implement token approval limits instead of using type(uint256).max to mitigate potential risks. (Found in both Curve2PoolAdapter and CurveTricryptoAdapter in the token approval sections)

b. External Contract Validation: Validate external contract addresses to ensure correctness and security. Introduce checks for the validity of indices provided by external contracts. (Curve2PoolAdapter and CurveTricryptoAdapter in the constructor sections)

c. Reentrancy Guards: Implement reentrancy guards in functions that call external contracts, even if not fully visible in the provided code, to prevent potential reentrancy attacks. (Not found explicitly, but a general recommendation for security)

d. Access Controls: Introduce access controls to restrict certain functions to authorized contracts or users. Define and enforce permissions for interactions with critical operations. (Both OceanAdapter and Ocean contracts)

e. Minimum Output Amounts: Reevaluate the use of 0 as the minimum output amount in Curve pool calls. Consider adjusting the minimum amounts to protect against slippage in various operations. (Curve2PoolAdapter and CurveTricryptoAdapter in primitiveOutputAmount functions)

f. Contract Upgradability: Consider making contracts upgradable to address potential vulnerabilities and allow for future improvements. Implement a secure upgrade mechanism if possible. (Ocean contract)

g. Event Logging: Enhance event logging to include detailed information for better on-chain tracking and auditing. (All contracts, especially in functions involving critical operations)

h. Fallback Function Safety: Ensure that fallback functions are secure and cannot inadvertently receive Ether. Perform a thorough review of fallback functions. (CurveTricryptoAdapter in the use of address(this).balance)

i. Validate External Dependencies: Before interacting with external contracts, validate the security and correctness of these contracts through audits or trusted sources. (Both Curve adapters)

j. Gas Optimization: Optimize gas usage, especially in functions that involve multiple interactions. Identify and eliminate any unnecessary or expensive operations. (Ocean contract in functions with loops and complex operations)

Codebase quality analysis

The codebase demonstrates a good understanding of Solidity best practices, but it requires additional attention to mitigate potential security risks. Thorough testing and auditing are recommended to ensure the robustness of the implementation.

Curve2PoolAdapter.sol:

Contract Functionality:

The Curve2PoolAdapter facilitates interactions with a Curve Finance 2pool through the Ocean platform. It handles swapping between two tokens, adding liquidity, and removing liquidity.

Security Issues:

1. Unlimited Token Approvals:

   • Issue: The contract uses type(uint256).max for token approvals, which may pose a security risk if approved contracts are compromised.
   • Recommendation: Implement a controlled approval mechanism with specific allowances rather than unlimited approvals.

2. Curve Pool Index Validation:

   • Issue: The contract assumes the correctness of Curve pool indices without validation, potentially leading to misconfigurations.
   • Recommendation: Include validation checks for Curve pool indices during contract initialization.

3. Slippage Risk:

   • Issue: The use of 0 as the minimum amount for swaps and liquidity operations in Curve pool calls could result in slippage risks.
   • Recommendation: Implement more robust slippage protection mechanisms, considering a minimum acceptable output amount.

4. External Contract Reliance:

   • Issue: The contract relies on the correctness of the Ocean platform and Curve pool contracts, exposing it to vulnerabilities in these contracts.
   • Recommendation: Thoroughly test and audit the interactions with external contracts to ensure robustness and security.

CurveTricryptoAdapter.sol:

Contract Functionality:

CurveTricryptoAdapter facilitates interactions with the Curve Tricrypto pool, allowing swapping between stablecoin, wrapped Bitcoin, and Ether, along with liquidity management.

Security Issues:

1. Primitive Address Validation:

   • Issue: The contract assumes that the provided primitive address is legitimate and secure, without validation.
   • Recommendation: Implement a validation check for the legitimacy and security of the provided Curve Tricrypto pool contract address.

2. Ether Balance Handling:

   • Issue: The use of address(this).balance to get the Ether balance should ensure no inadvertent receipt of Ether by other functions.
   • Recommendation: Review and ensure Ether balance handling is secure, preventing unintended transfers.

3. Reentrancy Vulnerability:

   • Issue: Lack of reentrancy guards may pose a vulnerability if external contracts called are malicious or compromised.
   • Recommendation: Implement reentrancy guards, especially for functions interacting with external contracts.

4. Trust in External Contracts:

   • Issue: The contract assumes trust in the primitive and Ocean contracts it interacts with without explicit validation.
   • Recommendation: Ensure that the primitive and Ocean contracts are trusted and audited to prevent potential security issues.

OceanAdapter.sol:

Contract Functionality:

OceanAdapter serves as an abstract contract for shell adapters in a DeFi platform. It interacts with an "Ocean" system and an external "primitive" specified at contract deployment.

Security Issues:

1. Unwrap Fee Calculation:

   • Issue: The unwrap fee calculation might result in integer division rounding errors, posing potential exploitable scenarios.
   • Recommendation: Enhance the unwrap fee calculation to address potential rounding issues and ensure precise calculations.

2. ERC1155 Token Handling:

   • Issue: The onERC1155Received function might pose a security risk if not designed to handle incoming ERC1155 tokens properly.
   • Recommendation: Review and enhance the onERC1155Received function for secure handling of incoming ERC1155 tokens.

Ocean.sol:

Contract Functionality:

Ocean is a part of a DeFi framework interacting with various token standards, enabling users to perform wrapping, unwrapping, and computing input/output amounts for trades.

Security Issues:

1. Reentrancy Risk:

   • Issue: While using status flags to prevent reentrancy, a comprehensive review of all external calls is essential.
   • Recommendation: Ensure all external calls are non-reentrant, especially during token transfers.

2. Decimal Handling Assumption:

   • Issue: The assumption that all ERC-20 tokens have a decimals() function could lead to reverts if a token deviates.
   • Recommendation: Implement additional checks to ensure the presence of the decimals() function in external tokens.

3. Fee Manipulation Power:

   • Issue: The contract owner's ability to change the unwrap fee divisor introduces potential fee manipulation risks.
   • Recommendation: Establish governance mechanisms or timelocks to regulate changes to critical parameters.

4. Token ID Validation:

   • Issue: The contract assumes the validity of provided token IDs for ERC-721 and ERC-1155 tokens.
   • Recommendation: Implement thorough validation checks for token IDs to prevent potential vulnerabilities.

5. Forwarder Trust:

   • Issue: Allowing forwarders to execute transactions introduces trust assumptions.
   • Recommendation: Encourage user education on forwarder selection and implement additional security measures.

6. Contract Upgradability:

   • Issue: The contract does not appear to be designed for upgradability.
   • Recommendation: Explore and implement strategies for contract upgradability to address potential vulnerabilities or incorporate improvements.

7. Gas Optimization:

   • Issue: The contract contains loops and potentially expensive operations.
   • Recommendation: Consider optimizations, such as reducing unnecessary iterations, for improved gas efficiency.

In summary, a detailed quality analysis of each contract highlights their functionalities, security issues, and recommendations for improvement. Addressing these recommendations will contribute to the overall robustness and security of the DeFi protocol.

Centralization Risks:

In the Ocean.sol contract, there is a notable centralization risk associated with the contract owner's authority to change the unwrap fee divisor without the presence of robust governance mechanisms. The unwrapFeeDivisor determines the fee charged for unwrapping tokens, and any sudden or unexpected changes by the contract owner could lead to economic imbalances or exploitation by malicious actors. To mitigate this risk, it is recommended to implement a governance system that involves token holders or a decentralized decision-making process to govern changes in critical parameters, ensuring a more democratic and secure protocol evolution.

Mechanism Review:

The mechanisms for token wrapping, unwrapping, and interaction execution appear sound, but improvements are needed in terms of security checks, access controls, and validation of external dependencies.

The Ocean.sol contract demonstrates a comprehensive mechanism for interacting with various token standards and external primitives. Key aspects include:

• Token Wrapping and Unwrapping: Users can seamlessly wrap and unwrap ERC-20, ERC-721, and ERC-1155 tokens within the Ocean ecosystem, facilitating interoperability.

• Fee Management: The contract incorporates a fee for unwrapping tokens, calculated as a fraction of the unwrap amount. The fee is rounded down, and when the unwrap amount is below the divisor, no fee is charged, providing a transparent and user-friendly fee structure.

• Token Normalization: ERC-20 token amounts are normalized to 18 decimal places, ensuring consistency in interactions between tokens with different decimal representations.

• Interaction Execution: Users can execute single or multiple interactions in one transaction, enhancing efficiency and reducing transaction costs.

• Reentrancy Guards: Custom status flags, such as _ERC1155InteractionStatus and _ERC721InteractionStatus, are employed to prevent reentrancy attacks during token transfers, contributing to the overall security of the contract.

• Event Logging: The contract emits events for significant actions, aiding in transparent and verifiable on-chain tracking of transactions.

• Forwarding Transactions: Approved forwarders can execute interactions on behalf of users, streamlining user interactions and facilitating batched transactions.

• Interface Support: The contract implements various interfaces, including IERC721Receiver and IERC1155Receiver, ensuring proper handling of token transfers according to standard specifications.

• Owner Privileges: The contract owner has the privilege to change the unwrap fee divisor, providing flexibility for protocol adjustments.

Systemic Risks:

The systemic risks mainly revolve around the correctness and security of external contracts, the potential for reentrancy attacks, and the need for robust access controls. Conducting a thorough audit and implementing the recommended architecture changes can help mitigate these risks.

The Ocean.sol contract exhibits certain systemic risks that warrant attention and further scrutiny:

• Reentrancy Risks: While the contract employs custom status flags to prevent reentrancy attacks, it is crucial to conduct a thorough analysis of all external calls to ensure they are non-reentrant. Any oversight in this aspect could lead to potential vulnerabilities.

• Decimal Handling Assumption: The contract assumes that all ERC-20 tokens have a decimals() function. If a token deviates from this standard, the contract may revert. It is essential to validate the presence of the decimals() function in external tokens to prevent unexpected failures.

• Fee Manipulation: The contract owner's ability to change the unwrap fee divisor introduces a potential risk of fee manipulation. Implementing governance mechanisms or timelocks for such changes is recommended to ensure a controlled and transparent adjustment process.

• Token ID Handling: For ERC-721 and ERC-1155 tokens, the contract assumes that provided token IDs are valid. A robust validation mechanism should be implemented to verify the existence and ownership of token IDs to prevent potential vulnerabilities.

• Forwarder Trust: Allowing forwarders to execute transactions on behalf of users introduces a trust assumption. Users must trust forwarders not to perform malicious actions, emphasizing the importance of careful forwarder selection and user education.

• Contract Upgradability: The contract does not appear to be explicitly designed for upgradability. In the event of identifying vulnerabilities, updating the contract may pose challenges, and considerations for upgradability should be explored.

Time spent:

12 hours