Beanstalk Part 2
Beanstalk
DeFiHardhat
35,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Missing check for the max/min price in the `LibChainlinkOracle.getPrice()`

tigerfrake

Summary

The LibChainlinkOracle.sol contract specially the getPrice() function uses the aggregator to get/call the latestRoundData(). The function should check for the min and max amount return to prevent some case happen, something like this:

https://solodit.xyz/issues/missing-checks-for-chainlink-oracle-spearbit-connext-pdf
https://solodit.xyz/issues/m-16-chainlinkadapteroracle-will-return-the-wrong-price-for-asset-if-underlying-aggregator-hits-minanswer-sherlock-blueberry-blueberry-git

If a case like LUNA happens then the oracle will return the minimum price and not the crashed price.

Vulnerability Details

function getPrice(
address priceAggregatorAddress,
uint256 maxTimeout
) internal view returns (uint256 price) {
IChainlinkAggregator priceAggregator = IChainlinkAggregator(priceAggregatorAddress);
// 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, maxTimeout)) {
return 0;
}
// Adjust to 6 decimal precision.
return uint256(answer).mul(PRECISION).div(10 ** decimals);
} catch {
// If call to Chainlink aggregator reverts, return a price of 0 indicating failure
return 0;
}
}

The function tries to get latest price data here:

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, maxTimeout)) {
return 0;
}
// Adjust to 6 decimal precision.
return uint256(answer).mul(PRECISION).div(10 ** decimals);

and returns a value given by uint256(answer).mul(PRECISION).div(10 ** decimals);

Within the function, various checks are made such as checking for an invalid roundId that is 0 as well as checking for invalid Timestamp or Answer by invoking checkForInvalidTimestampOrAnswer() which returns a boolean indicating the status of the above parameters as shown here:

function checkForInvalidTimestampOrAnswer(
uint256 timestamp,
int256 answer,
uint256 currentTimestamp,
uint256 maxTimeout
) private pure returns (bool) {
// Check for an invalid timeStamp that is 0, or in the future
if (timestamp == 0 || timestamp > currentTimestamp) return true;
// Check if Chainlink's price feed has timed out
if (currentTimestamp.sub(timestamp) > maxTimeout) return true;
// Check for non-positive price
if (answer <= 0) return true;
}

As it can be seen, this function only checks for non-positive price i.e if (answer <= 0) return true; but it does not check for the min/max price allowable and therefore when the getPrice() returns the price value, this value can be anything absurdly odd.

Impact

Without a check for minimum and maximum allowable prices, the function might return prices that are unreasonably high or low. If the function returns a price that is significantly outside the expected range, it could lead to financial losses for users or the system itself. For instance, if the price returned is much lower than the actual market price for an asset, users might end up selling at a loss, or if the price is excessively high, it could result in overpaying for assets.

Tools Used

Manual Review

Recommendations

As shown in this Documentation:
A circuit breaker should be implemented on the oracle so that when the price edges close to minAnswer or maxAnswer it starts reverting.

Configure your application to detect when the reported answer is close to reaching reasonable minimum and maximum limits so it can alert you to potential market events. Separately, configure your application to detect and respond to extreme price volatility or prices that are outside of your acceptable limits.

Some check like this can be added to avoid returning of the `min price or the max price in case of the price crashes.

require(answer < _maxPrice, "Upper price bound breached");
require(answer > _minPrice, "Lower price bound breached");
Updates

Lead Judging Commences

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

Chainlink validation

Support

FAQs

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