Platform: Code4rena
Start Date: 27/11/2023
Pot Size: $36,500 USDC
Total HM: 0
Participants: 22
Period: 8 days
Judge: 0xA5DF
Id: 308
League: ETH
Rank: 16/22
Findings: 1
Award: $44.92
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: Sathish9098
Also found by: 0xSmartContract, 0xepley, LinKenji, Myd, ZanyBonzy, albahaca, alexbabits, clara, foxb868, invitedtea, oakcobalt, peanuts
44.915 USDC - $44.92
Shell Protocol, a creation Cowri Labs Inc. ais a set of smart contracts which aims to revolutionize the DeFi landscape by embracing a modular approach. The innovative architecture simplifies smart contract development and enables users to batch multiple transactions, all while minimizing gas fees. Shell Protocol's clean, modular contracts streamline development and enhance user experience, heralding a new era of accessible, efficient DeFi.
The protcol comprises two major components the Ocean and the Proteus which work in tandem to deliver a powerful and adaptable DeFi ecosystem. The Ocean's shared accounting system facilitates the seamless integration of various DeFi primitives - AMMs, lending pools, algorithmic stablecoins, NFT markets etc. while Proteus's versatile AMM engine empowers users to create highly efficient custom pools. This synergy positions Shell Protocol as a transformative force in the DeFi landscape.
The Ocean is the main focus of this analysis. It is a single smart contract that serves as a central hub for the protocol's interactions. It simplifies the composition of DeFi primitives by providing a unified accounting system, a single ledger for all tokens types, and a standardized accounting framework. The Ocean also tracks intermediate balance updates in memory instead of storage to improve gas efficiency. This innovative approach makes Shell Protocol a versatile and efficient platform for building and using DeFi applications.
For the purpose of this audit, Shell v3 Protocol consists of four smart contracts totaling 993 SLoC. Its core design principle is composition, enabling efficient and flexible integration of various DeFi primitives. It utilizes six external calls to facilitate communication with other blockchain components. It is planned to be deployed on Arbitrum chain. The Ocean, the protocol's accounting contract, complies with the ERC1155 and ERC165 token standards. It operates independently of oracles and sidechains and represents an upgrade from its predecessor, Shell v2.
Like other codebases built using composition patterns, the Shell protocol's code was initially challenging to understand. Nonetheless, the code is thoroughly commented, providing detailed explanations for each function's functionality although, it doesnt fully follow NatSpec. The provided documentations, the whitepapers, are top tier. They go into great details about how the protocol works, how the Ocean interacts with other primitives. It explains the reasons for the decisions made in the codebases, and overall made the codebase more understandable.
Some basic contract safety measures also seemed to be ignored. Castings are done unsafely, there are non zero checks when making initializations, state variables are not reasonably capped, checks-effects-interaction pattern is ignored and so on. These are by no means very serious issues, but they cause a host of inconvienieces to the protocol implementation. One error on the part of the developers was using an Arbitrum incompatible solidity version, 0.8.20 - due to the PUSH0 code. Recommend switching to an earlier solidity version, preferably 0.8.10 so as to match the current V2 implementation.
Error handling is a bit odd, with the protocol implementing the outdated assert
errors. A mix of custom, assert and require errors is implemented. To promote uniformity across the codebase and also for best practice. we recommend updating these with custom errors preferabley, as they offer superior gas efficiency. Events are well handled, emitted for important parameter changes, although in some cases, they seemed to not follow the checks-effects-interaction patterns. This should be fixed for best practices and to protect from reentrancy. Also, to save gas, its recommended to use assembly to emit events.
The protocol demonstrates great test coverage, 98%. Hardhart and Foundry tests exist for the contracts. Unit and fuzzing tests are implemented for the codebase. More tests can be conducted, to improve coverage, uncover more rudimentary bugs, strengthen modularity, and elevate overall security measures.
The codebase in scope is divided into two major modules:
Accounting contract
Adapter contracts
OceanAdapter.sol - acts as a supporting component for other adapters within the protocol. It orchestrates the necessary transformations and interactions between the Ocean, the central accounting system, and various DeFi primitives. Notably, it shouldn't hold any tokens directly. It comprises 94 SLoC.
Curve2PoolAdapter.sol - facilitates integration between the protocol and the curve2pool, enabling users to perform token swaps, add liquidity, and remove liquidity for the curve usdc-usdt pool. It comprises 139 SLoC.
CurveTricryptoAdapter.sol - bridges the protocol with the curve tricrypto pool, empowering users to execute token swaps, inject liquidity, and withdraw liquidity for the curve usdt-wbtc-eth pool. It comprises 199 SLoC.
Centralization exists but not a lot, certain function are limited to the Ocean, Owner and ApprovedForwarders. It goes without saying that actions from these parties have impacts on the protocol. And they're vulnerable points of failure. One odd decision made by the team was to split the user and User ApprovedForwarder
functions into two when they essentially do the same thing (See Oceans.sol L210, L256, L229, L281). We think the team can find a work around for this.
ThirdParty dependencies is also a point of entry of attack to the protocol. The contract imports OZ contracts version 4.8.1 which has a number of known issues, this should be updated, as these vulnerabilities in these contracts are likely to have an effect in the system. Also, while not in scope, the OZ Ownable was used, which is not as safe as its counterpart Ownable2Step. Recommend updating to the latter instead. The curve adapters are major points of risks to the protocol. The CurveTricryptoAdapter
contract is the adapter for the Arbirtrum Tricrypto pool, which Curve Finance has deemed vulnerable and advised users to not use.
Smart Contract vulnerabilities also pose a significant threat to the security and integrity of decentralized protocols. These vulnerabilities can manifest as critical security flaws, enabling malicious actors to exploit the protocol and steal funds or manipulate the system. Logical inconsistencies in the code can also lead to unexpected behavior and unintended consequences, potentially causing financial losses or compromising the protocol's overall functionality. Constant sytem upgrades and audits are recommended, to keep the protocol up to date with latest technologies.
Other risks include chain reorg due to Arbitrum's optimistic rollup nature, non-standard ERC20 tokens, compromised primitives, economic attacks and so on
Documentation review - We reviewed the website, readme, whitepaper and explanations provided by the devs. While this was going on, we ran the contracts through static analyzer and compared the generated reports to the bot report and removed false positives.
Manual code review - Here, we manually reviewed the codebase, ran provided tests, tested out various attack vectors. We looked for ways to break the protocol invariants, while making sure the contracts comforom to the needed standarfs. We also tested out the functions' logic to make sure they work as intended.
Codebase comparisons - After the manual review, we studied similar protocol types, previous audits, previous commits and made comparisons to issues found in these protocols to see if the same mistakes were repeated and how effective the provided fixes were.
We made notes on the issues we found and prepared the needed reports.
The Shell v3 protocool aims to improve on the fundamentals developed in Shell v2, and intends to make the Ocean compatible with external protocols. It does this by introducing the OceanAdapter contracts which is a generalized adapter interface for adapter primitives.
In general, the protcol seems well designed, but identified risks need to be fixed. Measures should be implemented to protect the protocol from potential attacks. We recommend cleaning up the codebase and mititgating any identified vulnerabilities before deployment. This proactive approach will ensure the protocol's continued robustness and securit
30 hours
#0 - c4-pre-sort
2023-12-10T16:53:53Z
raymondfam marked the issue as sufficient quality report
#1 - c4-judge
2023-12-17T11:43:47Z
0xA5DF marked the issue as grade-b