Kelp DAO | rsETH - Ruhum's results

Lines of code

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

Vulnerability details

Impact

When a user mints rsETH, the price per share is determined by calculating the total value of all the assets held by the system. Before the user's shares are calculated, the DepositPool contract transfers the user's deposit amount to itself. That causes the asset amount to be inflated when the price per share is calculated which in turn decreases the shares the user receives.

Proof of Concept

When a user deposits assets into DepositPool, it'll first transfers those assets and then calculate the shares to mint:

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

To calculate the share price it uses the price of the asset and the price of rsETH which is the total value of all the underlying assets divided by the total supply of 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();
    }

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

        return totalETHInPool / rsEthSupply;
    }

Given that the DepositPool holds 100 stETH, has minted 99 rsETH, and 1 stETH = 1 ETH, then rsETH price = 100e18 * 1e18 / 99e18 = 1.010101e18

If a user now wants to deposit 1 stETH they should get: 1e18 * 1e18 / 1.010101e18 = 9.9000001e17.

But, because the deposit pool has first transferred the user's funds to itself, the rsETH price changes. Instead of holding 100 stETH it now holds 101 stETH: 101e18 * 1e18 / 99e18 = 1.020202e18. Thus, the user will receive 1e18 * 1e18 / 1.020202e18 = 9.8019804e17 rsETH.

The higher the deposit amount the higher the difference in rsETH minted.

Tools Used

none

Recommended Mitigation Steps

Calculate the user's rsETH shares before transferring the underlying asset to the deposit pool.

Assessed type

Math

Lines of code

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

Vulnerability details

Impact

The rsETH share price can be inflated by sending any of the underlying assets directly to the deposit pool. Subsequent depositors will receive no rsETH when they deposit unless the deposited amount is larger than the amount transferred to the deposit pool directly.

Because withdrawals aren't possible at the time of deployment the attack is very risky. You'd lock up your funds for an indefinite amount of time. If someone else deposits a large amount of funds, you also risk losing your deposit amount. I don't think the attack will be executed but it should be mentioned nonetheless.

Proof of Concept

1. Attacker deposits 1 wei worth of underlying assets to mint 1 rsETH
2. Attacker sends a large amount of underlying assets to the deposit pool contract, e.g. 1000 stETH
3. That will cause the rsETH price to be inflated. Anybody that deposits less than 1000 stETH worth of assets won't receive any rsETH and lose their deposit amount.

The rsETH price is calculated by the oracle using the total amount of underlying assets and their current value in ETH.

   /// @notice Provides RSETH/ETH exchange rate
    /// @dev calculates based on stakedAsset value received from eigen layer
    /// @return rsETHPrice exchange rate of RSETH
    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;
            }
        }

        return totalETHInPool / rsEthSupply;
    }

In our scenario, rsETH supply is 1, stETH supply is 1000e18 and stETH price in ETH is 1e18. We'd get 1000e18 * 1e18 / 1 = 1e39.

If someone deposits 999 stETH, they will get:

999e18 * 1e18 / 1e39 = 0.999 rsETH which solidity will round down to 0.

Tools Used

none

Recommended Mitigation Steps

There are multiple ways to prevent this attack. Uniswap, for example, burns the first LP shares in V2: https://github.com/Uniswap/v2-core/blob/master/contracts/UniswapV2Pair.sol#L121

Here's an article that gives a little more detail: https://mixbytes.io/blog/overview-of-the-inflation-attack

Assessed type

Math