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

13 stars 11 forks source link

Users will lose their funds due to lack of precision scaling #387

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/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTDepositPool.sol#L95-L110

Vulnerability details

Impact

Lack of precision scaling in getRsETHAmountToMint function for deposited asset will cause user's loss.

When user deposit an asset you make internal call to getRsETHAmountToMint function to calculate the amount of RsETH that should be minted for user but the problem is all of assets doesn't have 18 decimals and it might be different, the way you calculate the mint amount only works correctly for assets with 18 decimals otherwise if deposited asset have less than 18 decimals the returned value will be a small value which causes user's loss or on the other side if it is more than 18 decimals you will mint more tokens for caller.

Proof of Concept

Imagine a user wants to deposit 1 WBTC which has 8 decimals into the LRTDepositPool contract by calling depositAsset function and for calculating the RsETH amount getRsETHAmountToMint function will be called and this line of code will run :

File: 2023-11-kelp\src\LRTDepositPool.sol
109:  rsethAmountToMint = (amount * lrtOracle.getAssetPrice(asset)) / lrtOracle.getRSETHPrice();

the BTC/ETH price currently is 17849113669425485000 and lets consider the RsETH price is 1 ETH, so the calculation will look like this:

WBTC amount = 100000000
BTC/ETH price = 17849113669425485000
RsETH price = 1000000000000000000

rsethAmountToMint = (100000000 * 17849113669425485000) / 1000000000000000000 answer = 1784911366

As you see the value that will be minted for user is 0.0000000017 RsETH while he should receive 17.84 RsETH for depositing 1 WBTC

Tools Used

Manual Review

Recommended Mitigation Steps

In getRsETHAmountToMint function scale precision of deposited asset to 18 if it's decimals is not 18

    function getRsETHAmountToMint(
        address asset,
        uint256 amount
    )
        public
        view
        override
        returns (uint256 rsethAmountToMint)
    {
        // setup oracle contract
        address lrtOracleAddress = lrtConfig.getContract(LRTConstants.LRT_ORACLE);
        ILRTOracle lrtOracle = ILRTOracle(lrtOracleAddress);

+       uint8 decimals = IERC20(asset).decimals();
+       if(decimals != 18){
+          amount = (amount * 1e18) / (10 ** decimals);
+       }
        // calculate rseth amount to mint based on asset amount and asset exchange rate
        rsethAmountToMint = (amount * lrtOracle.getAssetPrice(asset)) / lrtOracle.getRSETHPrice();
    }

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

c4-judge commented 10 months ago

fatherGoose1 marked the issue as unsatisfactory: Invalid