Rounding error when depositing will cause loss of funds for caller

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

Significant loss of user funds, when calling depositAsset the user will lose their depositAmount and receive 0 RsETH in return due to a rounding area, for specific assets.

Proof of Concept

The protocol intends to use Chainlink Price Feeds as the price oracle however they haven't considered the fact that most price feeds denominated in USD return prices to 8 decimals.

This is problematic as RsETH is 18 decimals, therefore in getRsETHAmountToMint the returned amount to mint will likely round to 0 for these assets as you can see here.

        // calculate rseth amount to mint based on asset amount and asset exchange rate
        rsethAmountToMint =
            (amount * lrtOracle.getAssetPrice(asset)) /
            lrtOracle.getRSETHPrice(); 
    }

The value from getAssetPrice is computed as so:

function getAssetPrice(
        address asset
    ) public view onlySupportedAsset(asset) returns (uint256) {
        return IPriceFetcher(assetPriceOracle[asset]).getAssetPrice(asset);
    }

As you can see the value is used directly, without any consideration of the decimals.

Also, the depositAsset function doesn't allow the user to specify a minimum amount of RsETH to receive and there is no validation of the minted amount, meaning should this rounding error occur it will not be caught.

Tools Used

manual

Recommended Mitigation Steps

Account for the decimals from the returned value of the price feed, and allow users to specify a minimum amount of RsETH to receive.

Assessed type

Invalid Validation

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #97

[c4-pre-sort]

raymondfam marked the issue as duplicate of #479

[c4-judge]

fatherGoose1 changed the severity to 2 (Med Risk)

[c4-judge]

fatherGoose1 marked the issue as satisfactory

[c4-judge]

fatherGoose1 changed the severity to QA (Quality Assurance)

[c4-judge]

fatherGoose1 marked the issue as grade-b