Kelp DAO | rsETH - marchev's results

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/oracles/ChainlinkPriceOracle.sol#L38 https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTOracle.sol#L68-L71

Vulnerability details

Impact

The rsETH price calculation contains a vulnerability which allows users to deposit assets and receive rsETH of much higher value than the deposited assets.

Proof of Concept

In the Kelp protocol, when users deposit assets using LRTDeposit#depositAsset(), they get rsETH in exchange of their deposited assets.

The _mintRsETH() is called in depositAsset() to mint the new rsETH a user would receive. The amount of minted rsETH is based on the deposited asset's amount and exchange rate using getRsETHAmountToMint():

https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTDepositPool.sol#L108-L109

The rsETH price is calculated in LRTOracle#getRSETHPrice():

https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTOracle.sol#L52-L76

In essence, the rsETH price calculation is performed via the following formula:

where,

$A_{1, 2,\ ...\ ,i}$ = The total asset deposits for each asset

$P_{1, 2,\ ...\ ,i}$ = The price of each asset

However, a known fact is that the Chainlink oracle's latestAnswer() function can return 0 instead of an error if no answer has been reached. This flaw can be exploited.

For example, if the oracle erroneously reports 0 for an asset, a user depositing another asset might receive more rsETH than usual. The severity of this issue varies based on the assets in the protocol and which asset has the oracle error.

To illustrate, consider a scenario with 100 stETH, 100 rETH, and 100 cbETH in the protocol. For simplicity, let's assume the price for both rETH and cbETH is 1 ETH. If the oracle fails for stETH, reporting 0 as the price, and Alice deposits 50 rETH, she would get approximately 75.75e18 rsETH instead of the 50e18 she would receive under normal conditions.

where

$rETH\ Price = 1e18$

$rsETH\ Price = \frac{100 \times 0 + 100 \times 1e18 + 100 \times 1e18}{300} \approx 0.66e18$

Thus,

In the absence of an error with the Chainlink price oracle, Alice would have received:

The potential for exploitation here hinges on the asset mix in the protocol and the specific asset affected by the oracle error.

Tools Used

Manual review

Recommended Mitigation Steps

Revert if AggregatorInterface#latestAnswer() returns 0:

--- a/src/oracles/ChainlinkPriceOracle.sol
+++ b/src/oracles/ChainlinkPriceOracle.sol
@@ -35,7 +35,9 @@ contract ChainlinkPriceOracle is IPriceFetcher, LRTConfigRoleChecker, Initializa
     /// @param asset the asset for which exchange rate is required
     /// @return assetPrice exchange rate of asset
     function getAssetPrice(address asset) external view onlySupportedAsset(asset) returns (uint256) {
-        return AggregatorInterface(assetPriceFeed[asset]).latestAnswer();
+        uint256 assetPrice = AggregatorInterface(assetPriceFeed[asset]).latestAnswer();
+        require(assetPrice != 0, "Oracle price error");
+        return assetPrice;
     }

     /// @dev add/update the price oracle of any supported asset

Alternatively, use latestRoundData() from the Chainlink Data Feeds API.

Assessed type

Oracle