code-423n4 / 2023-11-kelp-findings

13 stars 11 forks source link

Initial depositor can manipulate the rsETH price, forcing future LST stakers to receive ZERO rsETH #557

Closed c4-submissions closed 11 months ago

c4-submissions commented 11 months ago

Lines of code

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

Vulnerability details

Impact

LRTDepositPool.depositAsset relies on getRsETHAmountToMint to determine the amount of rsETH to mint. The first deposit can mint a minimal number of rsETH then donate LST tokens to the pool to grossly manipulate the rsETH price. When later depositor stakes into the pool they will lose their fund due to precision loss.

Proof of Concept

The root cause of the vulnerability is that the depositAsset function allows users to mint a realy small amount of rsETH at the initial stage (when rsETH supply is zero), exposing the pool to the exchange-rate inflation attack. Let us walk through the issue with the following scenario:

  1. At initial pool condition, getRSETHPrice returns 1 ether. Alice initiates a LRTDepositPool.depositAsset call with a minimal amount of 1 stETH (price of approx. 1e18 ETH/stETH in base units) to mint roughly (1 * 1e18 / 1e18 = 1) rsETH.

    // File: src/LRTDepositPool.sol
    function getRsETHAmountToMint(
        ...
    @>1        rsethAmountToMint = (amount * lrtOracle.getAssetPrice(asset)) / lrtOracle.getRSETHPrice();
    }
  2. Alice proceeds to transfer 1e18 stETH directly to the LRTDepositPool contract so that getTotalAssetDeposits now reads as 1 + 1e18. As a result, the getRSETHPrice call is manipulated to return an extremely high value since totalETHInPool was pumped up while rsEthSupply remains small. Given assetER stays unchanged at 1e18, getRSETHPrice now reads as (1 + 1e18) * 1e18 / 1 ~= 1e36.

    // File: src/LRTOracle.sol
    function getRSETHPrice() external view returns (uint256 rsETHPrice) {
    uint256 totalAssetAmt = ILRTDepositPool(lrtDepositPoolAddr).getTotalAssetDeposits(asset);
    totalETHInPool += totalAssetAmt * assetER;
    ...
    @>2        return totalETHInPool / rsEthSupply;
    }
  3. Bob joins the pool after Alice with 0.99e18 stETH. Due to the inflated rsETH price, he receives ZERO rsETH in return due to precision loss.

    @>1 becomes: rsethAmountToMint = 0.99e18 (stETH) * 1e18 (ETH/stETH) / 1e36 ~= 0 rsETH

  4. As a result, Alice's 1 rsETH token now represents the entire stETH asset in the pool including the 0.99e18 stETH that belongs to Bob. Alice has effectively stolen Bob's fund. Furthermore, the rsETH price is now even pumped 2x higher since rsEthSupply stays the same while totalETHInPool has doubled, ensuring innevitable loss for future depositors.

Notice: Bob's deposit amount being higher than 0.99e18 will not save him from the loss since Alice can always sniff Bob transaction and frontruns him with the right donation amount described at step 2.

Tools Used

Manual Review

Recommended Mitigation Steps

Consider requiring a minimal amount of rsETH tokens to be minted for the first minter, so that the rsETH price can be more resistant to manipulation.

Assessed type

Math

c4-pre-sort commented 11 months ago

raymondfam marked the issue as sufficient quality report

c4-pre-sort commented 11 months ago

raymondfam marked the issue as duplicate of #42

c4-judge commented 10 months ago

fatherGoose1 changed the severity to 3 (High Risk)

c4-judge commented 10 months ago

fatherGoose1 marked the issue as satisfactory