code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Summary

The require check in the constructor of OracleConvert.sol reverts when the sum of 2 pricefeed decimals is equal or greater than 16.

This excludes almost all Chainlink pricefeeds and results in vastly overinflated price calculations where it does work.

Proof of Concept

Github Link

OracleConvert.sol function constructor

    require(CL_TOKENA.decimals() + CL_TOKENB.decimals() >= 16, "Decimals error");

The vast majority of Chainlink pricefeeds have 8 decimals and some, for example AMPL / USD, have 18 decimals.

The require checks the sum of the decimals of 2 pricefeeds and reverts if it is equal or larger then 16, thereby excluding 99% of all chainlink pricefeeds.

Furthermore, since the latestAnswer function (line 50-54) can only provide a correct calculation if the pricefeeds of both tokens are exactly 8 (due to having hardcoded 8 as decimals), it ensures that any price calculation that does pass the require check will produce an output vastly larger then the actual price.

OracleConvert.sol function constructor

  function latestAnswer() public view returns (int256) {
    uint priceA = uint(getAnswer(CL_TOKENA));
    uint priceB = uint(getAnswer(CL_TOKENB));
    return int(priceA * priceB / (10 ** (CL_TOKENB.decimals() + CL_TOKENA.decimals() - 8))); 
  }

Impact

The Chainlink pricefeeds do not work for the vast majority of pricefeeds and overinflate the price in the few cases where it does work. So both a Denial of Service and losses to users of the protocol where it does work.