The deposited amount is included in how rsEthAmountToMint is calculated and it should not. Second depositors get less rsETH shares than deserved.

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTDepositPool.sol#L136
https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTDepositPool.sol#L109
https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTOracle.sol#L70
https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTDepositPool.sol#L49
https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTDepositPool.sol#L79

Vulnerability details

Impact

All deposits, starting with the second one, incur a loss in the received rsETH amount.

Proof of Concept

LRTDepositPool::depositAsset helps users to stake LST in exchange for rsETH shares.
First the LST is transferedFrom user to depositPool and rsETH is minted to depositor.

...
        if (!IERC20(asset).transferFrom(msg.sender, address(this), depositAmount)) {
            revert TokenTransferFailed();
        }
        // interactions
        uint256 rsethAmountMinted = _mintRsETH(asset, depositAmount);

The amount of rsETH to mint is calculated by getRsETHAmountToMint as following:

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

lrtOracle.getRSETHPrice calls LRTDepositPool::getTotalAssetDeposits -> getAssetDistributionData to get LST assets amounts distributed among depositPool, NDCs and eigenLayer to calculate the rsETH/ETH exchange rate.

    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?

        // @audit includes the transferred amount from the depositor in the depositAsset() function 
        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;
            }
        }
    }

So rsETHPrice is calculated as: `rsETHPrice = totalAssetDeposits * assetPrice / rsEthSupply

The issue rely in including the current deposit amount in the rsETH/ETH price calculation and then in rsETH amounts to mint formula.

Let's take the following example:
Note: to simplify I approximate the LST/ETH ratio to 1e18.

• Alice (first depositor) deposit 1 ether LST. rsETH totalSupply is 0 => getRSETHPrice returns 1 ether.
  Alice will mint:
  rsEthAmountToMint = 1 ether * 1e18 / (1 ether * 1e18 / 1 ether)
  rsEthAmountToMint = 1 ether rsETH
• Bob deposit 10 ether LST and will mint:
  rsEthAmountToMint = 10 ether * 1e18 / ( (1 ether + 10 ether) * 1e18 /1 ether)
  rsEthAmountToMint = 10/11 ether rsETH

Alice deposited 1 ether LSD and got 1 ether rsETH
Bob deposited 10 ether LSD and got less than 1 ether rsETH.

Tools Used

Manual Review

Recommended Mitigation Steps

Update LRTDepositPool::depositAsset : save rsethAmountToMint, transfer assets, mint rsETH shares to user:

    function depositAsset(
        address asset,
        uint256 depositAmount
    )
        external
        whenNotPaused
        nonReentrant
        onlySupportedAsset(asset)
    {
        // checks
        if (depositAmount == 0) {
            revert InvalidAmount();
        }
        if (depositAmount > getAssetCurrentLimit(asset)) {
            revert MaximumDepositLimitReached();
        }
+       rsethAmountToMint = getRsETHAmountToMint(_asset, _amount);

        if (!IERC20(asset).transferFrom(msg.sender, address(this), depositAmount)) {
            revert TokenTransferFailed();
        }

        // interactions
-        uint256 rsethAmountMinted = _mintRsETH(asset, depositAmount);
+        address rsethToken = lrtConfig.rsETH();
+        IRSETH(rsethToken).mint(msg.sender, rsethAmountToMint);

        emit AssetDeposit(asset, depositAmount, rsethAmountMinted);
    }

Assessed type

Other

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #62

[c4-judge]

fatherGoose1 marked the issue as satisfactory

[c4-judge]

fatherGoose1 changed the severity to 2 (Med Risk)

[c4-judge]

fatherGoose1 changed the severity to 3 (High Risk)