Lack of Slippage Protection in Asset Deposits for Price-Volatile LSTs

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

The current implementation of the depositAsset function in the DeFi protocol assumes that the prices of assets like stETH, rETH, and cbETH are either stable or appreciating relative to ETH. This assumption overlooks the potential volatility and price drops of these assets against ETH. In the absence of slippage protection, users could be exposed to significant financial risk, especially in scenarios where the asset value declines sharply after initiating a deposit but before the transaction is executed. This gap can lead to unfavorable RSETH minting rates and potential economic losses for users. The lack of slippage protection could undermine user confidence in the protocol’s robustness and its ability to safeguard user interests under volatile market conditions.

This vulnerability is rated medium severity because, while it may not directly lead to immediate large-scale financial losses or exploits, it exposes users to significant risk in volatile market conditions and can impact the trust and reliability of the protocol.

Proof of Concept

Illustrative Scenario:

• A user initiates a deposit of stETH.

https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTDepositPool.sol#L116-L144

    /// @notice helps user stake LST to the protocol
    /// @param asset LST asset address to stake
    /// @param depositAmount LST asset amount to stake
    function depositAsset(
        address asset,
        uint256 depositAmount
    )
        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);

        emit AssetDeposit(asset, depositAmount, rsethAmountMinted);
    }

• Between the initiation and execution of the transaction, Lido's stETH's price drops significantly against ETH due to unforeseen slashings. LRTOracle.getRSETHPrice() returns a comparatively higher rsETHPrice than getAssetPrice(asset) because the prices of rETH and cbETH remain unaffected:

https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTOracle.sol#L60-L78

        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;
            }
        }

        return totalETHInPool / rsEthSupply;

• As a result, the user receives a substantially lower amount of RSETH than expected.

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

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

Tools Used

Manual

Recommended Mitigation Steps

Introduce a slippage tolerance mechanism in the depositAsset function. For instance, users could specify their minimum share of RSETH minted, and transactions falling below this threshold would be reverted.

Assessed type

Timing

[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