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

13 stars 11 forks source link

The initial deposit of an asset will always fail due to the fact that `getTotalAssetDeposits()` returns 0. #871

Closed c4-submissions closed 11 months ago

c4-submissions commented 1 year ago

Lines of code

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

Vulnerability details

Impact

The LRTDepositPool#depositAsset() function is employed for users to deposit supported assets. This function relies on LRTDepositPool#getTotalAssetDeposits() to determine the amount of rsETH to be minted. However, the absence of initial deposits in the protocol—whether in the deposit pool, Node Delegator, or in assets staked in EigenLayer—results in LRTDepositPool#getTotalAssetDeposits() always being 0. This vulnerability prevents the protocol from successfully receiving deposits from users.

The getTotalAssetDeposits() function does always return 0 due to the absence of initial deposits, it could prevent the protocol from successfully receiving deposits from users.

The amount of rsETH to be minted is calculated based on the total asset deposits, and this total is initially 0, then no rsETH would be minted when users deposit assets. This could effectively prevent the protocol from receiving deposits, as users would not receive any tokens in return for their deposits.

Proof of Concept


File: src/LRTDepositPool.sol
// @audit line 141 in depositAsset() calls _mintRsETH()
141:    uint256 rsethAmountMinted = _mintRsETH(asset, depositAmount);

File: src/LRTDepositPool.sol
// @audit line 152 in _mintRsETH() calls getRsETHAmountToMint() to get amount rsETH to mint 
151:    function _mintRsETH(address _asset, uint256 _amount) private returns (uint256 rsethAmountToMint) {
152:        (rsethAmountToMint) = getRsETHAmountToMint(_asset, _amount);

File: src/LRTDepositPool.sol
// @audit line 109 in getRsETHAmountToMint() calls lrtOracle.getRSETHPrice() to get the price
109:    rsethAmountToMint = (amount * lrtOracle.getAssetPrice(asset)) / lrtOracle.getRSETHPrice();

File: src/LRTOracle.sol
// @audit line 70 lrtOracle.getRSETHPrice() call LRTDepositPool.getTotalAssetDeposits() to get all combined value of deposited assets, but initially
// this will be 0
70:     uint256 totalAssetAmt = ILRTDepositPool(lrtDepositPoolAddr).getTotalAssetDeposits(asset);
  1. depositAsset()
    • Allows a user to deposit a supported asset into the deposit pool.
  2. _mintRsETH()
    • Internal function to mint the equivalent amount of rsETH based on the deposited asset.
  3. getRsETHAmountToMint()
    • Returns the amount of rsETH to mint based on the deposited asset amount.
  4. getRSETHPrice()
    • Returns the current price of rsETH in USD.
  5. getTotalAssetDeposits()
    • Returns the total amount of deposited assets in the deposit pool.
    • This will call getAssetDistributionData() to calculate the amount, but initially there will not be any deposited amount, and this will return 0

Finally, 109 line rsethAmountToMint = (amount * lrtOracle.getAssetPrice(asset)) / lrtOracle.getRSETHPrice();in getRsETHAmountToMint() throw division by zero error. This error will prevent users from depositing assets.

Tools Used

Manual Review

Recommended Mitigation Steps

To address the issue, consider adding assets into the deposit pool or delegator contract as an initial supply. This will enable the protocol to start receiving deposits from users successfully.

Assessed type

Token-Transfer

c4-pre-sort commented 1 year ago

raymondfam marked the issue as insufficient quality report

raymondfam commented 1 year ago

The initial asset deposit is transferred to the contract prior to minting:

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

        if (!IERC20(asset).transferFrom(msg.sender, address(this), depositAmount)) {
            revert TokenTransferFailed();
        }
c4-pre-sort commented 1 year ago

raymondfam marked the issue as primary issue

c4-judge commented 11 months ago

fatherGoose1 marked the issue as unsatisfactory: Invalid