DeFiHardhatFoundry
250,000 USDC
View results
Submission Details
Severity: high
Valid

Oracle ignores `lookback` value when consulting price

Summary

The LibChainlinkOracle intends to use the lookback parameter to return the Time-Weighted Average Price (TWAP) from the Chainlink Oracle over the past specified seconds. However, due to faulty logic, this value is never actually used.

Currently, the contract contains a logical error where it checks if the lookback is greater than zero. If lookback is greater than zero, it does not use the lookback value. Conversely, if lookback is zero, the contract incorrectly uses the getTwap function, which returns the first price fetched from the oracle instead of the intended TWAP.

Vulnerability Details

function getTokenPrice(
address priceAggregatorAddress,
uint256 maxTimeout,
uint256 lookback
) internal view returns (uint256 price) {
return
@> lookback > 0
@> ? getPrice(priceAggregatorAddress, maxTimeout)
@> : getTwap(priceAggregatorAddress, maxTimeout, lookback);
}

A 2nd issue is that there is no minimum value determined for the lookback. As the project will be deployed on L2, there is a chance that the block.timestamp can cause the project to use a lookback very low like 1 second.

Example on Arbitrum:

"block timestamps are usually set based on the sequencer's clock. Because there's a possibility that the sequencer fails to post batches on the parent chain (for example, Ethereum) for a period of time, it should have the ability to slightly adjust the timestamp of the block to account for those delays and prevent any potential reorganisations of the chain. To limit the degree to which the sequencer can adjust timestamps, some boundaries are set, currently to 24 hours earlier than the current time, and 1 hour in the future."

When calling sunrise, the following logic is used to set the lookback on LibWellMinting

(uint256[] memory ratios, uint256 beanIndex, bool success) = LibWell
@> .getRatiosAndBeanIndex(tokens, block.timestamp.sub(s.sys.season.timestamp));

As shown above, in a L2 the scenario where block.timestamp.sub(s.sys.season.timestamp) can result in a very low number in seconds. i.e: 1-10 secs or a This will impact the reliability of Beanstalk on the TWAP price returned from the Chainlink Oracle.

Notice that in LibUsdOracle the docs mention the following:

/**
* @dev Returns the price of a given token in in USD with the option of using a lookback. (Usd:token Price)
* `lookback` should be 0 if the instantaneous price is desired. Otherwise, it should be the
* TWAP lookback in seconds.
@> * If using a non-zero lookback, it is recommended to use a substantially large `lookback`
@> * (> 900 seconds) to protect against manipulation.
*/

There is no check in place to guarantee this protection against manipulation.

Impact

  • The faulty lookback parameter affects Beanstalk's reliance on accurate TWAP for its operations. This can cause incorrect pricing data, resulting in mispricing and potential financial risks.

  • The TWAP function's susceptibility to very low lookback values compromises price accuracy and increases the protocol's exposure to price manipulation.

Tools Used

Manual Review

Recommendations

  • Use lookback when its value is > 0.

  • Ensure the minimum value for lookback when its value is > 0.

// LibChainlinkOracle
function getTokenPrice(
address priceAggregatorAddress,
uint256 maxTimeout,
uint256 lookback
) internal view returns (uint256 price) {
+ if (lookback > 0 && lookback < MINIMUM_LOOKBACK_VALUE) lookback = MINIMUM_LOOKBACK_VALUE; // i.e ensure min 900 secs.
return
lookback > 0
- ? getPrice(priceAggregatorAddress, maxTimeout)
- : getTwap(priceAggregatorAddress, maxTimeout, lookback);
+ ? getTwap(priceAggregatorAddress, maxTimeout, lookback)
+ : getPrice(priceAggregatorAddress, maxTimeout);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Validated
Assigned finding tags:

getTokenPrice never gives TWAP

Support

FAQs

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