Summary

Hard coded loop back will never work for chain link that(getEthUsdPrice)

Vulnerability Details

In the process of minting fertilizer, Usdprice is gotten by checking for the ethUsd price using lookback scenarios; First, it uses loopback = 0….for the getPrice() Secondly, it uses looback > 0 for the getTwap(). The getTwap works as intended but the vulnerability lies when trying to get the TWAP chainlink price and uses looback as 0 and not greater than Zero.

Impact

TwapVariables memory t;
t.endTimestamp = block.timestamp.sub(lookback);
// Check if last round was more than lookback ago.
if (timestamp <= t.endTimestamp) {
return uint256(answer).mul(PRECISION).div(10 ** decimals);
} else {
t.lastTimestamp = block.timestamp;
// Loop through previous rounds and compute cumulative sum until
// a round at least lookback seconds ago is reached.
while (timestamp > t.endTimestamp) {
t.cumulativePrice = t.cumulativePrice.add(
uint256(answer).mul(t.lastTimestamp.sub(timestamp))
);
roundId -= 1;
t.lastTimestamp = timestamp;
(answer, timestamp) = getRoundData(priceAggregator, roundId);
if (checkForInvalidTimestampOrAnswer(
timestamp,
answer,
t.lastTimestamp,
maxTimeout
)) {
return 0;
}
}
t.cumulativePrice = t.cumulativePrice.add(
uint256(answer).mul(t.lastTimestamp.sub(t.endTimestamp))
);
return t.cumulativePrice.mul(PRECISION).div(10 ** decimals).div(lookback);
}
} catch {
// If call to Chainlink aggregator revert
The function will revert when end.timestamp is greater than timestamp of TWAP variables. In the secenerio
when chainlink precision is divided by the look back = 0, it will always revert and that TWAP function will never be
called.
There will be conflict between the two internal functions or will always return the current price of Ethereum in
USD directly without calculating a TWAP. This function is used in LibChainOracle, LibEthUsdOracle, LibUniswapOracle, LibWstethEthOracle and MockSeasonFacet contracts

Tools Used

Manual Review

Recommendations

To address this issue, you need to modify the code to differentiate between the getPrice() and getTwap() scenarios based on the value of lookback. When lookback is zero, the function should directly return the current price without attempting to calculate the TWAP and when it greater than zero it should only return TWAP price of the chainlink.