Ion Protocol Seaport, ezETH and rsETH Integration Audit

OpenZeppelin Security | April 24, 2024

Table of Contents

Summary

Type
DeFi
Timeline
From 2024-03-20
To 2024-04-02
Languages
Solidity
Total Issues
10 (9 resolved)
Critical Severity Issues
0 (0 resolved)
High Severity Issues
1 (1 resolved)
Medium Severity Issues
0 (0 resolved)
Low Severity Issues
3 (2 resolved)
Notes & Additional Information
6 (6 resolved)
Client Reported Issues
0 (0 resolved)

Scope

We audited the Ion-Protocol/ion-seaport repository at commit 54110bb. In scope were the following files:

 src
├── SeaportBase.sol
├── SeaportDeleverage.sol
├── SeaportLeverage.sol
└── interfaces
    ├── IGemJoin.sol
    ├── IIonPool.sol
    ├── ISpotOracle.sol
    ├── IUFDMHandler.sol
    └── IWhitelist.sol

Additionally, we audited the Ion-Protocol/ion-protocol repository at commit 0a8c0d2. For files below labelled with "diff", only changes since the 1c42b94 commit were considered.

 src
├── Constants.sol (diff)
├── flash
│   ├── UniswapFlashswapDirectMintHandlerWithDust.sol
│   └── lrt
│       ├── EzEthHandler.sol
│       └── RsEthHandler.sol
├── interfaces
│   └── ProviderInterfaces.sol (diff)
├── libraries
│   └── lrt
│       ├── KelpDaoLibrary.sol
│       └── RenzoLibrary.sol
└── oracles
    ├── reserve
    │   └── lrt
    │       ├── EzEthWstEthReserveOracle.sol
    │       └── RsEthWstEthReserveOracle.sol
    └── spot
        └── lrt
            ├── EzEthWstEthSpotOracle.sol
            └── RsEthWstEthSpotOracle.sol

System Overview

The audited code within the "ion-seaport" repository centers around two contracts, the SeaportLeverage and SeaportDeleverage contracts. In the leverage case, a user is able to obtain collateral, deposit it into Ion and borrow against it, using the proceeds of the borrow to pay back the Seaport order creator. The deleverage case works in reverse, allowing a user to obtain the borrow asset, repay their borrow in Ion, and withdraw their collateral to repay the Seaport order creator. These contracts simply fulfill orders on Seaport, which must be created outside of the audited codebase prior to fulfilling them.

Both contracts utilize function selector clashing to hijack the transferFrom call that Seaport makes during order fulfillment. Within the "leverage" case, this is used to deposit tokens into Ion pools and borrow assets, which are then used to fulfill the remainder of the order. In the "deleverage" case, this is performed in reverse.

It should be noted that the Seaport protocol is complex, and thus various precautions are taken by Ion to constrain the order parameters and prevent unexpected behavior. The "leverage" functionality is limited to only whitelisted users, and both the "leverage" and "deleverage" functions can only be used to change the balances of the caller's own account.

The audited code within the "ion-protocol" repository implements the necessary contracts to allow Renzo "ezETH" and KelpDAO "rsETH" as collateral assets in the Ion system. There are new contracts which are the "handlers" and the "libraries" for these collaterals. For ezETH, there is a new version of the UniswapFlashswapDirectMintHandler contract called UniswapFlashswapDirectMintHandlerWithDust, which exists to help mitigate the problem of "ezETH" not being mintable in exact amounts. Due to the mathematics within ezETH, minting can only occur at certain values, and it is very likely that a user will not be able to receive exactly a specified amount out. Ion has also asked OpenZeppelin to investigate the error incurred in ezETH minting calculations, and some informal recommendations have been provided outside of this audit report. Contracts which price ezETH and rsETH against wstETH, the intended the borrow asset, have also been added. These contracts determine liquidation thresholds and maximal borrow amounts. They rely on internal state of the ezETH and rsETH contracts, as well as Chainlink and Redstone oracles, to compute fair values for both collaterals.

Integrations

Ion Protocol integrates with many other protocols deployed on Ethereum. As such, it is generally assumed that those protocols behave as documented. While there are many integrations associated with Ion, the integrations relevant to this specific engagement are:

  • Chainlink oracles for pricing ETH and wstETH
  • Redstone oracles for pricing ezETH and rsETH
  • Uniswap for flash-swaps to achieve leveraging positions in Ion
  • Seaport for resolving orders used to leverage positions in Ion
  • ezETH and its related contracts, such as the "Renzo Oracle" and "Renzo Restake Manager".
  • rsETH and its related contracts, such as the "rsETH LRT Oracle", "rsETH LRT Config" and "rsETH LRT Deposit Pool".
  • stETH and wstETH, where wstETH is the base token for all lending pools currently in Ion.

Security Model and Privileged Roles

As noted above, it is assumed that integrated contracts behave as described in their respective documentation. More specifically, we make the following assumptions:

  • All collaterals will continue to operate as described in their various smart contracts, without the addition of new functionality. For example, rsETH has the ability to accept new assets in exchange for minting rsETH, but for the purposes of this engagement it is assumed they will not.
  • It is assumed that all collaterals will slowly increase in ETH value, and certain parameters such as TVL or exchange rates within the collateral contracts will reflect this accurately.
  • It is assumed that values stored in collateral contracts will behave reasonably and remain similar to what they are now. For example, the TVL within ezETH, at time of writing, is about 550,000 ETH. It is assumed that this value will remain of a similar order of magnitude (for example, above 1000 ETH).
  • It is assumed that, should collateral values drop relative to the borrow asset (wstETH), they will do so slowly enough that liquidations can be performed given block space availability.
  • It is assumed that the "base" asset will remain wstETH for new markets.
  • It is assumed that Seaport 1.5 will be the version which Ion integrates with for the Seaport leveraging contracts.
  • It is assumed that all oracle updates from Chainlink or Redstone will be timely and accurate.
  • It is assumed that all contract addresses specified in Constants.sol are accurate and will match the values used in the deployed Ion contracts.
  • The operator of an account is able to borrow, repay and used approved tokens of the account.
  • The Seaport "leverage" and "deleverage" contracts must be set as a user's operator to alter the balance of the user's vault.
  • Only whitelisted borrowers can use the Seaport "leverage" function.

Monitoring Recommendations

We encourage the Ion team to consider implementing monitoring for their deployed contracts.

  • Consider monitoring for any upgrades to the Renzo contracts, including all those defined in Constants.sol. If an upgrade is detected, scan all execution flows which Ion depends on for any changes in the Renzo contract logic. Additionally, identify all changes to logic pertaining to values like TVL or ezETH total supply, which Ion also depends on.
  • Consider monitoring for any upgrades to the KelpDAO contracts (for rsETH). If an upgrade is detected, scan all execution flows which Ion depends on for any changes in the rsETH contract logic. Additionally, identify all changes to logic pertaining to values like rsEthPrice (from the RSETH_LRT_ORACLE) which Ion also depends on.
  • Establish a plan to pause the Ion protocol or change specific parameters, such as the spot address for a given collateral, in the event of an upgrade to any collateral contracts.
  • Monitor for upgrades to any tokens which are utilized in the Seaport leveraging or deleveraging contracts.
  • Monitor for changes to ezETH's TVL and total supply, and identify maximal reasonable error for a predefined max deposit size. Consider the value of this error as compared to current gas prices on Ethereum mainnet to determine whether logic within the EzEthHandler should be updated.
  • Monitor for significant drops in the TVL of ezETH or rsETH, to identify smart contract bugs within collaterals quickly. Create a plan for pausing either the Ion protocol or modifying parameters for specific markets.
  • Monitor collateral prices using various sources such as Uniswap, Curve, and Coingecko to identify price disparities for collateral assets. These may signal smart contract bugs or "black swan" market conditions. Establish plans for pausing or modifying market parameters based on certain thresholds being reached. For example, "pause all markets if the price for a collateral asset on Uniswap diverges from the price on Curve by more than 10% for more than 100 blocks".
  • Monitor all transactions utilizing the Seaport leveraging contracts, specifically checking for token transfer events. Seaport is a complicated system, so special attention should be payed to it. Ensure that tokens are moving as intended. Consider removing the "SeaportLeverage" contract from the protocol whitelist if unexpected behavior is detected, to prevent losses for users.

Additionally, before performing certain actions, we encourage Ion to validate the following:

  • Before integrating new tokens to the Seaport leveraging contracts, generally ensure that these tokens do not make calls to external contracts, as these could interfere with correct operation of the Seaport leveraging contracts. Pay special attention to "before transfer" and "after transfer" hooks on tokens.

Special considerations for ezETH

The Ion team specifically requested a review of the math used for ezETH integration within the Ion protocol. The audit team has relayed their findings regarding this asset to Ion outside of this audit report. In summary, the findings were that the minting of ezETH cannot necessarily result in any arbitrary amount specified.

Generally, minting behavior of ezETH is determined by the intermediate value, inflationPercentage, which is used for computing the TVL growth of a deposit into ezETH and the subsequent growth of the supply of ezETH. Since inflationPercentage has a maximal value of 1e18 - 1, there are thus that many different values for the amount of ezETH which can be minted. There are possibly many more potential values for the amount of ETH which can be deposited, and thus the inflationPercentage is the limiting factor in potential values of ezETH minted.

We encourage the Ion team to assess the tradeoff between increased gas costs and potential savings to users when deciding to make computations more accurate. Consider the typical values of deposits into the Ion protocol and utilize these to compute potential error as a general range of values. In addition, extensively document the error considerations to reassure users who may be concerned about inaccuracy in the computations.

 

High Severity

Full Deleverage Functionality Can Be Permanently Bricked

In the seaportCallback4878572495 function within the SeaportDeleverage contract, there is a requirement that the BASE token balance of the contract is 0 after a full deleverage.

However, this will always revert if a user has independently transferred any amount of BASE tokens into the SeaportDeleverage contract. Since there is no way to transfer the tokens out of the contract, a single transfer will permanently brick the contract.

Consider removing this check, or modifying it to instead check that the token balance of the contract has not changed since the beginning of the execution flow. Alternatively, consider either modifying the logic to transfer all unneeded BASE tokens to the user, or implementing a "rescue" function to remove stranded BASE tokens.

Update: Resolved in pull request #3. The sanity check was removed to prevent bricking of the contract.

Low Severity

Incomplete Docstrings

Throughout the codebase there are several parts that have incomplete docstrings. For instance:

Consider thoroughly documenting all functions/events (and their parameters or return values) that are part of any contract's public API. When writing docstrings, consider following the Ethereum Natural Specification Format (NatSpec).

Update: Resolved in pull request #76.

Floating Pragma

Pragma directives should be fixed to clearly identify the Solidity version with which the contracts will be compiled.

Throughout the codebase there are multiple floating pragma directives. For instance, the files IGemJoin.sol, IIonPool.sol, IUFDMHandler.sol, IWhitelist.sol have the solidity ^0.8.4 floating pragma directive.

Consider using a fixed pragma version.

Update: Acknowledged, not resolved. The Ion Protocol team stated:

The floating pragma was kept in order to allow other projects using a different Solidity version to use this repo as a submodule.

Inconsistent Use of ILK_INDEX in Seaport Leveraging Contracts

The SeaportDeleverage contract uses an immutable ILK_INDEX which is set in the constructor of the inherited SeaportBase contract.

There is an inconsistency in that SeaportDeleverage assumes an index of 0 in many places in both the deleverage and seaportCallback4878572495 functions, while SeaportLeverage.sol uses ILK_INDEX. This inclusion of the ILK_INDEX variable suggests that the contract should work with any ilk. However, the SeaportDeleverage contract would not function if the selected ilk index was not 0.

Consider using a consistent Ilk Index across the "Leverage" and "Deleverage" contracts.

Update: Resolved in pull request #4. The SeaportDeleverage contract was modified to use a hardcoded ILK_INDEX of 0.

Notes & Additional Information

Incremental Update Could be Wrapped in an Unchecked Block

Since Solidity version 0.8.0, any arithmetic operation automatically checks for over- and underflows, which cost gas. Since it is highly unlikely that a positively incrementing variable will overflow within a loop, the increment could be wrapped into an unchecked block.

This applies to the following instances:

To improve gas consumption, consider wrapping the incremental update into an unchecked block to save the gas required to check against overflows.

Update: Resolved in pull request #74. The incremented update was removed in an unrelated refactoring.

Unused Errors

Throughout the codebase, there are unused errors. For instance:

To improve the overall clarity, intentionality, and readability of the codebase, consider either using or removing any currently unused errors.

Update: Resolved in pull request #74 and pull request #5.

Unused Import

In SeaportDeleverage.sol, the import of IWhitelist.sol is unused and can be removed.

Consider removing unused imports to improve the overall clarity and readability of the codebase. Alternatively, if it is intended to use the whitelist within SeaportDeleverage.sol, consider implementing it.

Update: Resolved in pull request #5.

Lack of Security Contact

Providing a specific security contact (such as an email or ENS name) within a smart contract significantly simplifies the process for individuals to communicate if they identify a vulnerability in the code. This practice proves beneficial as it permits the code owners to dictate the communication channel for vulnerability disclosure, eliminating the risk of miscommunication or failure to report due to lack of knowledge on how to do so. Additionally, if the contract incorporates third-party libraries and a bug surfaces in these, it becomes easier for the maintainers of those libraries to make contact with the appropriate person about the problem and provide mitigation instructions.

Consider adding a NatSpec comment containing a security contact on top of the contracts definition. Using the @custom:security-contact convention is recommended as it has been adopted by the OpenZeppelin Wizard and the ethereum-lists.

Update: Resolved in pull request #5 for the ion-seaport repository and in pull request #76 for the ion-protocol repository.

Unchecked transfer/transferFrom Call

Some early implementations of ERC-20 tokens would return false instead of reverting the transaction when a transfer failed. This behavior is considered unsafe because a false return value could be ignored by the caller, leading to unexpected behavior.

In SeaportDeleverage.sol, the transfer call is unchecked.

Consider to use SafeERC20 OpenZeppelin library to perform safe token transfers. Note that with the current application, only ETH staking derivatives that revert on transfer are being called with transfer. However, in the case that the code is modified to support other "base" assets in the future, this will protect against unexpected behavior during transfer calls.

Update: Resolved in pull request #5.

Typographical Errors

The following typographical errors have been identified:

Consider correcting the typos for better readability and a more professional codebase.

Update: Resolved in pull request #76 and pull request #5.

 
 

Conclusion

One high-severity vulnerability was found in the contracts which were quickly acknowledged by the Ion team. Both the ion-protocol and ion-seaport codebases were covered by unit and fuzz tests. The Renzo ETH contracts were designed to deposit the "optimal" amount such that no excess ETH is deposited for any amount of ezETH. We evaluated that Ion implemented this correctly. However, this optimised amount makes a very small difference compared to imperfect deposit amounts. The Ion team was very helpful in documenting and explaining the codebase and clarifying the system's design.

 

Appendix

Issue Classification

OpenZeppelin classifies smart contract vulnerabilities on a 5-level scale:

  • Critical
  • High
  • Medium
  • Low
  • Note/Information

Critical Severity

This classification is applied when the issue’s impact is catastrophic, threatening extensive damage to the client's reputation and/or causing severe financial loss to the client or users. The likelihood of exploitation can be high, warranting a swift response. Critical issues typically involve significant risks such as the permanent loss or locking of a large volume of users' sensitive assets or the failure of core system functionalities without viable mitigations. These issues demand immediate attention due to their potential to compromise system integrity or user trust significantly.

High Severity

These issues are characterized by the potential to substantially impact the client’s reputation and/or result in considerable financial losses. The likelihood of exploitation is significant, warranting a swift response. Such issues might include temporary loss or locking of a significant number of users' sensitive assets or disruptions to critical system functionalities, albeit with potential, yet limited, mitigations available. The emphasis is on the significant but not always catastrophic effects on system operation or asset security, necessitating prompt and effective remediation.

Medium Severity

Issues classified as being of a medium severity can lead to a noticeable negative impact on the client's reputation and/or moderate financial losses. Such issues, if left unattended, have a moderate likelihood of being exploited or may cause unwanted side effects in the system. These issues are typically confined to a smaller subset of users' sensitive assets or might involve deviations from the specified system design that, while not directly financial in nature, compromise system integrity or user experience. The focus here is on issues that pose a real but contained risk, warranting timely attention to prevent escalation.

Low Severity

Low-severity issues are those that have a low impact on the client's operations and/or reputation. These issues may represent minor risks or inefficiencies to the client's specific business model. They are identified as areas for improvement that, while not urgent, could enhance the security and quality of the codebase if addressed.

Notes & Additional Information Severity

This category is reserved for issues that, despite having a minimal impact, are still important to resolve. Addressing these issues contributes to the overall security posture and code quality improvement but does not require immediate action. It reflects a commitment to maintaining high standards and continuous improvement, even in areas that do not pose immediate risks.

Related Posts