c4-submissions commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTDepositPool.sol#L119 https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTDepositPool.sol#L151 https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTDepositPool.sol#L95 https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTOracle.sol#L52 https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTDepositPool.sol#L47

Vulnerability details

Impact

The miscalculation of the RsETH price leads to users receiving less than their rightful RsETH in depositAsset().

Proof of Concept

Within the depositAsset() function, when users deposit assets to mint RsETH, the rsethAmountMinted is incorrectly calculated. The process involves transferring a supported asset into LRTDepositPool.sol and then calling _mintRsETH().

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

        // interactions
        uint256 rsethAmountMinted = _mintRsETH(asset, depositAmount);

The supported asset is transferred into the LRTDepositPool, then _mintRsETH() function calculates rsethAmountToMint using getRsETHAmountToMint() based on the specific _asset and its corresponding _amount.

        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();

Price of RsETH is calculated by total combined ETH value of all supported assets divide by RsRTH totalSupply. The root cause of this issue is in the calculation of RsETH price via lrtOracle.getRSETHPrice(), where it iterates through supported assets to compute totalETHInPool.

        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;

Back in LRTDepositPool.getTotalAssetDeposits(), it combines the total asset present in protocol. The assets are located in 3 different contracts, LRTDpositPool.sol, NodeDelegator.sol and its corresponding EigenLayer strategy. This returns incorrect amount as user's asset was previously transferred into LRTDeposit.sol before calculating RsETH price. This miscalculation results in an inflated RsETH price, leading to a reduced rsethAmountToMint.

    function getTotalAssetDeposits(address asset) public view override returns (uint256 totalAssetDeposit) {
        (uint256 assetLyingInDepositPool, uint256 assetLyingInNDCs, uint256 assetStakedInEigenLayer) =
            getAssetDistributionData(asset);
        return (assetLyingInDepositPool + assetLyingInNDCs + assetStakedInEigenLayer);
    }

Tools Used

Manual Review

Recommended Mitigation Steps

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

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

        emit AssetDeposit(asset, depositAmount, rsethAmountMinted);
    }

Assessed type

Context

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 #62

c4-judge commented 11 months ago

fatherGoose1 marked the issue as satisfactory

c4-judge commented 10 months ago

fatherGoose1 changed the severity to 2 (Med Risk)