getRSETHPrice() could be manipulated by donating LSTs to LTRDepositPool

[c4-submissions]

Lines of code

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#L109 https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTDepositPool.sol#L79

Vulnerability details

Impact

An attacker could skew the RSETH price by donating LSTs (stETH, rETH, cbETH) to LTRDepositPool. The getRSETHPrice() calls ILRTDepositPool.getTotalAssetDeposits(asset) in a loop for each supported asset as follows.

    function getRSETHPrice() external view returns (uint256 rsETHPrice) {
        ...
        for (uint16 asset_idx; asset_idx < supportedAssetCount;) {
            ...
            uint256 totalAssetAmt = ILRTDepositPool(lrtDepositPoolAddr).getTotalAssetDeposits(asset);
            totalETHInPool += totalAssetAmt * assetER;

            unchecked {
                ++asset_idx;
            }
        }

        return totalETHInPool / rsEthSupply;
    }

Inside getTotalAssetDeposits(), the call to getAssetDistributionData(asset) returns the current balance of LST of LTRDepositPool inside the assetLyingInDepositPool variable.

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

    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?
        assetLyingInDepositPool = IERC20(asset).balanceOf(address(this));
        ...
    }

Therefore, the RSETH price returned from getRSETHPrice() could be dramatically increased by a donation attack.

And an attacker, as a first depositor, can block further deposits to LTRDepositPool by forcing subsequent depositors to mint 0 RSETH tokens.

The attack scenario is the following:

1. The attacker, as the first depositor, calls depositAsset() with 1 wei of stETH as depositAmount and mints 1 wei of RSETH.
2. The attacker donates 1 stETH to LTRDepositPool.
3. So the price returned from getRSETHPrice() will be 1 ether * 10^18 instead of 1 ether.
4. A benign depositor will mint 0 RSETH because denominator is huge in the formula (amount * lrtOracle.getAssetPrice(asset)) / lrtOracle.getRSETHPrice().

Proof of Concept

• 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#L109
• https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTDepositPool.sol#L79

Tools Used

Manual review.

Recommended Mitigation Steps

Use ERC4626.sol as a base contract for the LTRDepositPool which has built-in protection.

Assessed type

DoS

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #42

[c4-judge]

fatherGoose1 marked the issue as satisfactory