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

13 stars 11 forks source link

Risk of Minting Zero RsETH Tokens for Small Asset Deposits #315

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#L119 https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTDepositPool.sol#L151 https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTDepositPool.sol#L95 https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTOracle.sol#L52

Vulnerability details

Impact

Users are at risk of receiving zero RsETH when supplying a small enough amount of supported assets during the minting process, potentially resulting in the loss of their assets to other stakers.

Proof of Concept

The minting of RsETH occurs through the depositAsset() function, where users supply supported assets to receive corresponding RsETH tokens. Within this process, _mintRsETH() determines the rsethAmountToMint using getRsETHAmountToMint().

    function _mintRsETH(address _asset, uint256 _amount) private returns (uint256 rsethAmountToMint) {
        (rsethAmountToMint) = getRsETHAmountToMint(_asset, _amount);

        address rsethToken = lrtConfig.rsETH();
        // mint rseth for user
        IRSETH(rsethToken).mint(msg.sender, rsethAmountToMint);
    }

The calculation of rsethAmountToMint within getRsETHAmountToMint() is determined as follows: the ETH value of the provided asset divided by the RsETH price.

        rsethAmountToMint = (amount * lrtOracle.getAssetPrice(asset)) / lrtOracle.getRSETHPrice();

The RsETH price is derived from the aggregated ETH value of all supported assets in the protocol divided by the RsETH totalSupply.

        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;

Currently protocol supports stETH, rETH and cbETH. The issue arises when a user, prompted by a significantly lower price of the supported assets compared to the other 2, attempts to mint a notably small amount. In such instances, the calculation for rsethAmountToMint can potentially return zero due to division truncation, resulting in the user receiving no stake of the total pool, while their asset is transferred into the protocol.

Tools Used

Manual Review

Recommended Mitigation Steps

The protocol should consider minimum depositAmount in depositAsset() or revert when rsethAmountToMint == 0

    function _mintRsETH(address _asset, uint256 _amount) private returns (uint256 rsethAmountToMint) {
        (rsethAmountToMint) = getRsETHAmountToMint(_asset, _amount);
+       if(rsethAmountToMint == 0) revert();
        address rsethToken = lrtConfig.rsETH();
        // mint rseth for user
        IRSETH(rsethToken).mint(msg.sender, rsethAmountToMint);
    }

Assessed type

Math

c4-pre-sort commented 11 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 11 months ago

raymondfam marked the issue as duplicate of #83

c4-judge commented 10 months ago

fatherGoose1 marked the issue as unsatisfactory: Invalid