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

13 stars 11 forks source link

price of RsETH is not calculated correctly #393

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/LRTOracle.sol#L70-L71

Vulnerability details

Impact

In getRSETHPrice function the precision of total balance of assets isn't converted to 18 decimals, if an asset has less decimals it cause the returned price of RSETH become larger than expected.

File: \2023-11-kelp\src\LRTOracle.sol
68:         uint256 assetER = getAssetPrice(asset);
...
70:         uint256 totalAssetAmt = ILRTDepositPool(lrtDepositPoolAddr).getTotalAssetDeposits(asset);
71:         totalETHInPool += totalAssetAmt * assetER;
72: 
...
78:         return totalETHInPool / rsEthSupply;

the assetER has 18 decimals and also rsEthSupply has 18 decimals so if totalAssetAmt has different decimals the calculation for the price on line 78 will be wrong, for assets which has less than 18 decimals, makes the price become larger and for assets which has greater than 18 decimals the price will be smaller.

in this function you add different values with different decimals into totalETHInPool.

Proof of Concept

Imagine there is 2000 USDT (USDT has 6 decimals) in the pool and there is 1 RsETH as total supply

If we want to call getRSETHPrice with these values the formated version of this calculation will look like this:

assetER for USDT/ETH = 488704664632542
totalAssetAmt of USDT = 2000000000 (it has 6 decimals)
totalETHInPool = 2000000000 * 488704664632542 = 977409329265084000000000
rsEthSupply = 1000000000000000000

return 977409329265084000000000 / 1000000000000000000 = 977409

As you see the price will be 977409 which is too small

Tools Used

Manual review

Recommended Mitigation Steps

Consider scale precision of the totalAssetAmt into 18

        for (uint16 asset_idx; asset_idx < supportedAssetCount;) {
            address asset = supportedAssets[asset_idx];
            uint256 assetER = getAssetPrice(asset);

            uint256 totalAssetAmt = ILRTDepositPool(lrtDepositPoolAddr).getTotalAssetDeposits(asset);
+           uint8 decimals = IERC20(asset).decimals();
+           if(decimals != 18){
+               totalAssetAmt = (totalAssetAmt * 1e18) / (10 ** decimals);
+           }
            totalETHInPool += totalAssetAmt * assetER;

            unchecked {
                ++asset_idx;
            }
        }

        return totalETHInPool / rsEthSupply;
    }

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