search

code-423n4 / 2023-07-moonwell-findings

1 stars 0 forks source link

The vulnerability in the `scalePrice` function is due to the lack of precision protection during division, potentially resulting in rounding errors and inaccurate scaled prices. #377

Open code423n4 opened 11 months ago

code423n4 commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-07-moonwell/blob/8694244ebf607a4ed33c0b74f422019fe8eb8d3e/src/core/Oracles/ChainlinkCompositeOracle.sol#L212

Vulnerability details

Impact

The vulnerability in the scalePrice function is related to potential rounding errors in the division operation when scaling prices. Although the contract has overflow protection, it lacks precision protection during division, which can result in inaccurate scaled prices. This occurs because Solidity's integer division behavior truncates the fractional part of the result, leading to incorrect calculations.

Returning a zero price from the oracle could have serious impact on the protocol.

Proof of Concept

price / (10 uint256(priceDecimals - expectedDecimals)).toInt256(); = 100 / (10 uint256(18 - 2)).toInt256(); = 100 / (10 ** uint256(16)).toInt256(); = 100 / 10000000000000000; = 0;

Tools Used

Manual / VSC.

Recommended Mitigation Steps

To mitigate this vulnerability, fixed-point arithmetic or a library like SafeMath should be used to handle division, ensuring accurate scaling without rounding errors. By using fixed-point arithmetic, fractional numbers can be represented accurately, maintaining precision during arithmetic operations.

Assessed type

Math

c4-pre-sort commented 11 months ago

0xSorryNotSorry marked the issue as primary issue

ElliotFriedman commented 11 months ago

expected decimals is always 18, so this is not an issue.

c4-sponsor commented 11 months ago

ElliotFriedman marked the issue as sponsor disputed

alcueca commented 11 months ago

This is a great spot to plug in this article of mine: https://hackernoon.com/getting-prices-right

price in the PoC matches the base token decimals, which are 18 like in WETH. We are aiming to get a price in the quote token, that having 2 decimals must be GUSD. This means that we need to remove 16 decimals from the price, and any price below 0.01 GUSD/WETH will be rounded down to zero.

While this is a fault of GUSD, it does mean that certain asset pairs won't work because of the difference in decimals and relative values. In that sense, this is valid QA.

c4-judge commented 11 months ago

alcueca changed the severity to QA (Quality Assurance)

c4-judge commented 11 months ago

alcueca marked the issue as grade-a