The price of rsEHT could be manipulated by the first staker

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/c5fdc2e62c5e1d78769f44d6e34a6fb9e40c00f0/src/LRTDepositPool.sol#L95-L110
https://github.com/code-423n4/2023-11-kelp/blob/c5fdc2e62c5e1d78769f44d6e34a6fb9e40c00f0/src/LRTOracle.sol#L52-L79

Vulnerability details

Impact

The first staker can potentially manipulate the price of rsETH through a donation attack, causing subsequent stakers to receive no rsETH after depositing. The first staker can exploit this method to siphon funds from other users.

Proof of Concept

The mining amount of rsETH is calculated in function getRsETHAmountToMint which directly utilizes the total value of the asset divided by the price of a single rsETH.

    function getRsETHAmountToMint(
        address asset,
        uint256 amount
    )
        public
        view
        override
        returns (uint256 rsethAmountToMint)
    {
        // setup oracle contract
        address lrtOracleAddress = lrtConfig.getContract(LRTConstants.LRT_ORACLE);
        ILRTOracle lrtOracle = ILRTOracle(lrtOracleAddress);

        // calculate rseth amount to mint based on asset amount and asset exchange rate
        rsethAmountToMint = (amount * lrtOracle.getAssetPrice(asset)) / lrtOracle.getRSETHPrice();
    }

Subsequently, the price of rsETH is related to its totalSupply and the total value of deposited assets.

    function getRSETHPrice() external view returns (uint256 rsETHPrice) {
        address rsETHTokenAddress = lrtConfig.rsETH();
        uint256 rsEthSupply = IRSETH(rsETHTokenAddress).totalSupply();

        if (rsEthSupply == 0) {
            return 1 ether;
        }

        uint256 totalETHInPool;
        address lrtDepositPoolAddr = lrtConfig.getContract(LRTConstants.LRT_DEPOSIT_POOL);

        address[] memory supportedAssets = lrtConfig.getSupportedAssetList();
        uint256 supportedAssetCount = supportedAssets.length;

        for (uint16 asset_idx; asset_idx < supportedAssetCount;) {
            address asset = supportedAssets[asset_idx];
            uint256 assetER = getAssetPrice(asset);

            uint256 totalAssetAmt = ILRTDepositPool(lrtDepositPoolAddr).getTotalAssetDeposits(asset);
            totalETHInPool += totalAssetAmt * assetER;

            unchecked {
                ++asset_idx;
            }
        }
//@audit the price of rsETH is calculated based on the asset and totalSupply
        return totalETHInPool / rsEthSupply;
    }

The total value of deposited assets comprises three parts: the assets in LRTDepositPool, the assets in NodeDelagator, and the assets in the eigenlayer. Anyone can directly contribute asset tokens to LRTDepositPool or NodeDelegator to augment the total value of deposited assets.

    function getAssetDistributionData(address asset)
        public
        view
        override
        onlySupportedAsset(asset)
        returns (uint256 assetLyingInDepositPool, uint256 assetLyingInNDCs, uint256 assetStakedInEigenLayer)
    {
        // Question: is here the right place to have this? Could it be in LRTConfig?
        assetLyingInDepositPool = IERC20(asset).balanceOf(address(this));

        uint256 ndcsCount = nodeDelegatorQueue.length;
        for (uint256 i; i < ndcsCount;) {
            assetLyingInNDCs += IERC20(asset).balanceOf(nodeDelegatorQueue[i]);
            assetStakedInEigenLayer += INodeDelegator(nodeDelegatorQueue[i]).getAssetBalance(asset);
            unchecked {
                ++i;
            }
        }
    }

Case

Therefore, the price of rsETH is susceptible to manipulation by the first staker, considering the following scenario:

• 1. Alice is the first staker and she deposits 1 USDC (the price of USDC is set to $1), she will get 1 wei rsETH, and the totalSupply of rsETH is 1 wei.
     Here is the test, add it to test/LRTDepositPoolTest.t.sol and run with forge test --match-test test_ControlPrice -vv.

diff --git a/test/LRTDepositPoolTest.t.sol b/test/LRTDepositPoolTest.t.sol
index 40abc93..63349c2 100644
--- a/test/LRTDepositPoolTest.t.sol
+++ b/test/LRTDepositPoolTest.t.sol
@@ -9,10 +9,11 @@ import { ILRTDepositPool } from "src/interfaces/ILRTDepositPool.sol";

 import { TransparentUpgradeableProxy } from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";
 import { ProxyAdmin } from "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol";
+import "forge-std/console.sol";

 contract LRTOracleMock {
     function getAssetPrice(address) external pure returns (uint256) {
-        return 1e18;
+        return 1;
     }

     function getRSETHPrice() external pure returns (uint256) {
@@ -109,6 +110,23 @@ contract LRTDepositPoolDepositAsset is LRTDepositPoolTest {
         lrtDepositPool.depositAsset(rETHAddress, 2 ether);
     }

+    function test_ControlPrice() external {
+        vm.startPrank(alice);
+
+        // alice balance of rsETH before deposit
+        uint256 aliceBalanceBefore = rseth.balanceOf(address(alice));
+
+        rETH.approve(address(lrtDepositPool), 1 ether);
+        lrtDepositPool.depositAsset(rETHAddress, 1 ether);
+
+        // alice balance of rsETH after deposit
+        uint256 aliceBalanceAfter = rseth.balanceOf(address(alice));
+        vm.stopPrank();
+
+        console.log(" rsETH of Alice: ", aliceBalanceAfter - aliceBalanceBefore);
+
+    }
+
     function test_DepositAsset() external {
         vm.startPrank(alice);

• 2. Alice donates 10000 USDC to the LRTDepositPool to inflate the price of rsETH. Now the price of rsETH is: (10000 + 1)ether / 1 wei = 10001 ether
• 3. Any subsequent staker who deposits assets worth less than 10001 USDC will not receive any rsETH, and they won't be able to withdraw the deposited assets either. Alice can directly siphon off these funds.
     For example, if Bob deposit 10000 USDC, then the rsethAmountToMint is (10000 ether * 1) / (10001)ether = 0

rsethAmountToMint = (amount * lrtOracle.getAssetPrice(asset)) / lrtOracle.getRSETHPrice();

Moreover, there is no check on the actual amount of rsETH received by the user, and the execution continues even if this amount is zero.

    function _mintRsETH(address _asset, uint256 _amount) private returns (uint256 rsethAmountToMint) {
        (rsethAmountToMint) = getRsETHAmountToMint(_asset, _amount);

        address rsethToken = lrtConfig.rsETH();
        // mint rseth for user
        //@audit sender could receive 0 token
        IRSETH(rsethToken).mint(msg.sender, rsethAmountToMint);
    }

Tools Used

Manual Review

Recommended Mitigation Steps

It is recommended to pre-mint some rsETH tokens to prevent price manipulation or ensure that the rsethAmountToMint is greater than zero.

Assessed type

Invalid Validation

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as primary issue

[raymondfam]

Could have used one of the LST's in the POC instead of USDC.

[c4-pre-sort]

raymondfam marked the issue as high quality report

[c4-sponsor]

gus-staderlabs marked the issue as disagree with severity

[gus-stdr]

We agree this is an issue. We also agree that it should be of a MEDIUM severity as it is an edge case that happens on the first protocol interaction.

[c4-sponsor]

manoj9april (sponsor) confirmed

[fatherGoose1]

Judging as HIGH. While it is an edge case, the potential loss of funds is present. Vault donation attacks have been judged as high in the majority of contests where no safeguards are implemented.

[c4-judge]

fatherGoose1 marked the issue as satisfactory

[c4-judge]

fatherGoose1 marked the issue as selected for report

[manoj9april]

Initial minting is a way of mitigating this issue. And this mitigation could be done after deployment. Hence no safeguard were added in contract. Hence request to decrease to medium.

[fatherGoose1]

Initial minting is a way of mitigating this issue. And this mitigation could be done after deployment. Hence no safeguard were added in contract. Hence request to decrease to medium.

Based on the implementation, this issue will remain HIGH. Funds are at risk until Kelp takes subsequent action to mitigate.