code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-08-goodentry/blob/4b785d455fff04629d8675f21ef1d1632749b252/contracts/helper/LPOracle.sol#L67-L104

Vulnerability details

Summary

The protocol bases its price calculations on Chainlink pricefeeds to always have 8 decimals. However, Chainlink pricefeeds with 18 decimals do exist and break the calculations.

Proof of Concept

Github Link

LPOracle.sol function latestAnswer

    /*
      The normalised positions (18 decimals) are multiplied with the chainlink value (8 decimals), giving val.
      val is divided by LP_TOKEN.totalSupply(), which has 18 decimals, and casted to an int
      The return value represents the value * 10**8 of a single LP token 
    */
    require(decimalsA <= 18 && decimalsB <= 18, "Incorrect tokens");
    uint val = norm_a * priceA * 10**(18-decimalsA) + norm_b * 10**(18-decimalsB) * priceB;
    return int(val / LP_TOKEN.totalSupply());

When a pricefeed with 18 decimals is used, for example AMPL / USD feed decimals are 18, the uint val calculation would be completed distorted since it would become 10**(18-18) = 1 and provide an output magnitudes smaller than the actual price.

Impact

A distortion of oracle price calculations leads to incorrect value assessments and losses to the users of the protocol.

Tools Used

Manual review

Recommendations

Adapt the logic to account for varying amounts of decimal tokens.

Assessed type

Decimal

141345 commented 1 year ago

seems invalid

“8 decimals for USD” and “18 decimals for ETH”

whether to add AMPL pool is up to the admin

c4-pre-sort commented 1 year ago

141345 marked the issue as primary issue

c4-sponsor commented 1 year ago

Keref marked the issue as sponsor disputed

c4-judge commented 1 year ago

gzeon-c4 changed the severity to QA (Quality Assurance)