code-423n4 / 2023-11-kelp-findings

13 stars 11 forks source link

user can manipulate rseth price by transferring asset directly for profit #588

Closed c4-submissions closed 11 months ago

c4-submissions commented 11 months ago

Lines of code

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

Vulnerability details

Impact The vulnerability allows users to manipulate the rseth price by directly transferring assets to the deposit pool or node delegator contracts. By increasing the totalAssetAmt used in the price calculation, users can artificially inflate the rseth price and potentially earn profits from this manipulation. Proof of Concept For example, let's assume the current asset price is denoted as p. Now, a user transfers an amount x of assets to the deposit pool. User's cost for the transferred assets would be p*x. Before the transfer, the rseth total supply is denoted as t, and the price of rseth is represented as rp. Here is the code rseth price can be inflated.

    function getRSETHPrice() external view returns (uint256 rsETHPrice) {
        address rsETHTokenAddress = lrtConfig.rsETH();
        uint256 rsEthSupply = IRSETH(rsETHTokenAddress).totalSupply();

        if (rsEthSupply == 0) {
            return 1 ether;
        }
        // ......
        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;
    }

After the transfer, the new rseth price, denoted as rsethPrice_new, can be calculated using the following formula: rsethPrice_new = rp * (1 + (p*x) / t) Here is the code snippet of getAssetDistributionData function. Both assetLyingInDepositPool and assetLyingInNDCs can be manipulated.

    assetLyingInDepositPool = IERC20(asset).balanceOf(address(this));
        uint256 ndcsCount = nodeDelegatorQueue.length;
        for (uint256 i; i < ndcsCount;) {
            assetLyingInNDCs += IERC20(asset).balanceOf(nodeDelegatorQueue[i]);
            assetStakedInEigenLayer += INodeDelegator(nodeDelegatorQueue[i]).getAssetBalance(asset);
            unchecked {
                ++i;
            }
        }

If the getRSETHPrice() function is used as the rseth price oracle, the user can easily manipulate the rseth price by directly transferring assets to the deposit pool. By transferring a specific amount of assets, the user can influence the calculation and potentially earn profits from the manipulated rseth price.

Recommended Mitigation Steps Use custom storage to store the asset balance instead of simple IERC20.balanceOf

Assessed type

Other

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

c4-judge commented 10 months ago

fatherGoose1 marked the issue as satisfactory