c4-submissions commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTDepositPool.sol#L136 https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTOracle.sol#L70

Vulnerability details

Impact

When depositAsset, the price of RsETH is related to the depositAmount. The larger the depositAmount is, the higher the price of RsETH is, and the less RsETH quantity can be obtained. this is unfair to users who deposit a lot of assets at once, and the price of RsETH is higher than the actual expected price because the assets in the pool are calculated in advance.

LRTDepositPool#depositAsset:

    function depositAsset(address asset, uint256 depositAmount)

Proof of Concept

LRTDepositPool#depositAsset,transfer asset first, then mint RsETH:

  function depositAsset(
        address asset,
        uint256 depositAmount
    )
        external
        whenNotPaused
        nonReentrant
        onlySupportedAsset(asset)
    {
        .....
@>      if (!IERC20(asset).transferFrom(msg.sender, address(this), depositAmount)) {
            revert TokenTransferFailed();
        }
@>      uint256 rsethAmountMinted = _mintRsETH(asset, depositAmount);
        emit AssetDeposit(asset, depositAmount, rsethAmountMinted);
    }

LRTOracle#getRSETHPrice, totalETHInPool include the asset that is currently being deposited,the value of rsEthSupply is before deposit.

  function getRSETHPrice() external view returns (uint256 rsETHPrice) {
        address rsETHTokenAddress = lrtConfig.rsETH();
        uint256 rsEthSupply = IRSETH(rsETHTokenAddress).totalSupply();
        .....
        for (uint16 asset_idx; asset_idx < supportedAssetCount;) {
            address asset = supportedAssets[asset_idx];
            uint256 assetER = getAssetPrice(asset);
            //@audit totalAssetDeposit include the asset that is currently being deposited
@>          uint256 totalAssetAmt = ILRTDepositPool(lrtDepositPoolAddr).getTotalAssetDeposits(asset);
            totalETHInPool += totalAssetAmt * assetER;
            unchecked {
                ++asset_idx;
            }
        }
@>      return totalETHInPool / rsEthSupply;
    }

Tools Used

vscode manual

Recommended Mitigation Steps

Get the price of RsETH before transferring the asset (calculate the number of Mint RsETH)

-    if (!IERC20(asset).transferFrom(msg.sender, address(this), depositAmount)) {
-        revert TokenTransferFailed();
-    }
-    uint256 rsethAmountMinted = _mintRsETH(asset, depositAmount);

+    (rsethAmountToMint) = getRsETHAmountToMint(_asset, _amount);
+    if (!IERC20(asset).transferFrom(msg.sender, address(this), depositAmount)) {
+        revert TokenTransferFailed();
+    }
+    IRSETH(address(lrtConfig.rsETH())).mint(msg.sender, rsethAmountToMint);

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

c4-judge commented 10 months ago

fatherGoose1 marked the issue as satisfactory