Beanstalk Part 1
Beanstalk
DeFiHardhatOracleProxyUpdates
100,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

`getEthUsdTwap()` is a flawed implementation and would almost always fail for a reasonable `lookBack` value

igdbaxe

Summary

Vulnerability Details

Take a look at https://github.com/Cyfrin/2024-02-Beanstalk-1/blob/a3658861af8f5126224718af494d02352fbb3ea5/protocol/contracts/libraries/Oracle/LibChainlinkOracle.sol#L70-L136

function getEthUsdTwap(uint256 lookback) internal view returns (uint256 price) {
// First, try to get current decimal precision:
uint8 decimals;
try priceAggregator.decimals() returns (uint8 _decimals) {
// If call to Chainlink succeeds, record the current decimal precision
decimals = _decimals;
} catch {
// If call to Chainlink aggregator reverts, return a price of 0 indicating failure
return 0;
}
​
// Secondly, try to get latest price data:
try priceAggregator.latestRoundData() returns (
uint80 roundId,
int256 answer,
uint256 /* startedAt */,
uint256 timestamp,
uint80 /* answeredInRound */
) {
// Check for an invalid roundId that is 0
if (roundId == 0) return 0;
if (checkForInvalidTimestampOrAnswer(timestamp, answer, block.timestamp)) {
return 0;
}
​
uint256 endTimestamp = block.timestamp.sub(lookback);
// Check if last round was more than `lookback` ago.
if (timestamp <= endTimestamp) {
return uint256(answer).mul(PRECISION).div(10 ** decimals);
} else {
uint256 cumulativePrice;
uint256 lastTimestamp = block.timestamp;
// Loop through previous rounds and compute cumulative sum until
// a round at least `lookback` seconds ago is reached.
while (timestamp > endTimestamp) {
cumulativePrice = cumulativePrice.add(
uint256(answer).mul(lastTimestamp.sub(timestamp))
);
roundId -= 1;
try priceAggregator.getRoundData(roundId) returns (
uint80 /* roundId */,
int256 _answer,
uint256 /* startedAt */,
uint256 _timestamp,
uint80 /* answeredInRound */
) {
if (checkForInvalidTimestampOrAnswer(_timestamp, _answer, timestamp)) {
return 0;
}
lastTimestamp = timestamp;
timestamp = _timestamp;
answer = _answer;
} catch {
// If call to Chainlink aggregator reverts, return a price of 0 indicating failure
return 0;
}
}
cumulativePrice = cumulativePrice.add(
uint256(answer).mul(lastTimestamp.sub(endTimestamp))
);
return cumulativePrice.mul(PRECISION).div(10 ** decimals).div(lookback);
}
} catch {
// If call to Chainlink aggregator reverts, return a price of 0 indicating failure
return 0;
}
}

This function is used to calculate the TWAP ETH/USD price from the Chainlink oracle over the past lookback seconds.

Case is with the fact that this function loops through the seconds, keep in mind that normal Twap values should be around 30 minutes, which means that this function would attempt querying latestROundData() a whooping 1800 times which would must surely revert.

Impact

Protocol's core functionality not working as intended, any reasonable value for lookback would cause a crazy amount of gas and inevitably leading to a revert of this instance https://github.com/Cyfrin/2024-02-Beanstalk-1/blob/a3658861af8f5126224718af494d02352fbb3ea5/protocol/contracts/libraries/Oracle/LibEthUsdOracle.sol#L61-L63... and eventually all contract logic that route their call via this method.

Tools Used

Recommendations

Consider reimplementing how TWAP is used in protocol, best to use other oracle sources for TWAPs than chainlink

Updates

Lead Judging Commences

giovannidisiena Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

Informational/Invalid

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.