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

13 stars 11 forks source link

Developer assumption on decimals can lead to incorrect rsethAmountToMint calculation in future #285

Open c4-submissions opened 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#L109

Vulnerability details

Impact

Developer assumption on decimals can lead to incorrect rsethAmountToMint calculation in future and lead to lose of funds

Proof of Concept

https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTDepositPool.sol#L109 docs.chain.link/data-feeds/price-feeds/addresses?network=ethereum If future added LST's oracles won't be the same 18 decimals as rsETH then rsethAmountToMint will be calculated incorrect and will lead to lose of funds. Chailink oracle can report different decimals for different tokens. Usage of hardcoded decimals is incorrect because new LST tokens can be added in future by addNewSupportedAsset() function.

Tools Used

Manual review

Recommended Mitigation Steps

Call "function decimals() external view returns (uint8);" on asset chainlink oracle before calculations to choose which decimals precision to use

Assessed type

Decimal

c4-pre-sort commented 11 months ago

raymondfam marked the issue as sufficient quality report

c4-pre-sort commented 11 months ago

raymondfam marked the issue as duplicate of #97

c4-pre-sort commented 11 months ago

raymondfam marked the issue as duplicate of #479

c4-judge commented 10 months ago

fatherGoose1 changed the severity to 2 (Med Risk)

c4-judge commented 10 months ago

fatherGoose1 marked the issue as satisfactory

c4-judge commented 10 months ago

fatherGoose1 changed the severity to QA (Quality Assurance)

c4-judge commented 10 months ago

fatherGoose1 marked the issue as grade-b