`LRTDepositPool.depositAsset` function is missing minimum amount parameter (lacks slippage protection)

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

• In LRTDepositPool.depositAsset function : when a user deposits a supported LST asset in the deposit pool, he will be minted share rsETH tokens proportional to the deposited amount and the asset price.

• And as per the the price calculations, the depositor will almost get an amount of share tokens (rsETH) equivalent to the amount of the deposited asset: LRTDepositPool.getRsETHAmountToMint function/L109

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

  where lrtOracle.getRSETHPrice() is totalETHInPool / rsEthSupply.

• But this function is missing the slippage parameter (minimum amount of share tokens to be minted) that ensures the user gets a fair amount of shares (as the rsETH price calculation might be affected if the deposit pool receives a direct LST transfers without going through the depositing process-minting of rsETH).

Proof of Concept

LRTDepositPool._mintRsETH function

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

        address rsethToken = lrtConfig.rsETH();
        // mint rseth for user
        IRSETH(rsethToken).mint(msg.sender, rsethAmountToMint);
    }

LRTDepositPool.getRsETHAmountToMint function/L109

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

Tools Used

Manual Review.

Recommended Mitigation Steps

In depositAsset function: add a _minShareAmount parameter as a slippage protection:

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

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

        // interactions
        uint256 rsethAmountMinted = _mintRsETH(asset, depositAmount);
+       require(rsethAmountMinted > _minShareAmount,"calculated less than required by the user");

        emit AssetDeposit(asset, depositAmount, rsethAmountMinted);
    }

Assessed type

Context

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #39

[c4-pre-sort]

raymondfam marked the issue as duplicate of #148

[c4-judge]

fatherGoose1 marked the issue as satisfactory