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

Incorrect function logic for fetching price from Chainlink Oracle

Summary

When user calls gm, gm calculates the deltaB by fetching price from Chainlink Oracle. According to the comment of 'LibChainlinkOracle:getTokenPrice: 'Use lookback = 0 for the instantaneous price. lookback > 0 for a TWAP', however the code shows if lookback > 0, get instantaneous price, else get TWAP. Therefore instantaneous price is fetched when lookback > 0 (the difference between the current block timestamp and the timestamp of the start of the current season > 0) and the token is not C.WETH and C.WSTETH. This could lead to an incorrect calculation on the value of deltaB and the subsequent calculations, caseId and stepsun, which depend on deltaB.

https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/libraries/Oracle/LibChainlinkOracle.sol#L35-L47

Vulnerability Details

Step 1: User calls gm, and gm calls stepOracle for determining deltaB
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/beanstalk/sun/SeasonFacet/SeasonFacet.sol#L55

Step 2: The value of lookback is calculated by block.timestamp.sub(s.sys.season.timestamp), representing the time difference between the current block timestamp and the timestamp of the start of the current season.
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/libraries/Minting/LibWellMinting.sol#L157

Step 3: Then get usd price from Oracle LibUsdOracle.getUsdPrice in LibWell:getRatiosAndBeanIndex
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/libraries/Well/LibWell.sol#L50

Step 4: LibChainlinkOracle.getTokenPrice in LibUsdOracle:getTokenPriceFromExternal is triggered when LibUsdOracle:getUsdPrice is called and the token is not C.WETH and C.WSTETH.
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/libraries/Oracle/LibUsdOracle.sol#L62
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/libraries/Oracle/LibUsdOracle.sol#L122
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/libraries/Oracle/LibUsdOracle.sol#L155

Step 5. Get the token price from Chainlink. If lookback > 0, get instantaneous price, else get TWAP.
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/libraries/Oracle/LibChainlinkOracle.sol#L35-L47

Step 6. Subsequent calculations, caseId and stepsun, which depend on deltaB.
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/beanstalk/sun/SeasonFacet/SeasonFacet.sol#L56
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/beanstalk/sun/SeasonFacet/SeasonFacet.sol#L58

Impact

  1. Incorrect Functionality: The intended purpose of the function is not fulfilled.

  2. Misleading/Incorrect Price: Instantaneous price is fetched when lookback > 0 and the token is not C.WETH and C.WSTETH.

  3. Inaccurate Calculations: This could lead to inaccurate calculations of deltaB and the subsequent caseId and stepsun calculations which depend on deltaB (Similar case: M-04. Temperature and caseId are incorrectly adjusted when oracle fails - https://codehawks.cyfrin.io/c/2024-02-Beanstalk-1/results?t=report&lt=contest&sc=reward&sj=reward&page=1).

Tools Used

Manual review

Recommendations

Follow correct logic like below:
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/libraries/Oracle/LibEthUsdOracle.sol#L49-L64

* Use `lookback = 0` for the instantaneous price. `lookback > 0` for a TWAP.
* Return value has 6 decimal precision.
* Returns 0 if the ETH/USD Chainlink Oracle is broken or frozen.
**/
function getEthUsdPrice(uint256 lookback) internal view returns (uint256) {
return
lookback > 0
? LibChainlinkOracle.getTwap(
C.ETH_USD_CHAINLINK_PRICE_AGGREGATOR,
LibChainlinkOracle.FOUR_HOUR_TIMEOUT,
lookback
)
: LibChainlinkOracle.getPrice(
C.ETH_USD_CHAINLINK_PRICE_AGGREGATOR,
LibChainlinkOracle.FOUR_HOUR_TIMEOUT
);

Update the code to

* Use `lookback = 0` for the instantaneous price. `lookback > 0` for a TWAP.
function getTokenPrice(
address priceAggregatorAddress,
uint256 maxTimeout,
uint256 lookback
) internal view returns (uint256 price) {
return
lookback > 0
- ? getPrice(priceAggregatorAddress, maxTimeout)
- : getTwap(priceAggregatorAddress, maxTimeout, lookback);
+ ? getTwap(priceAggregatorAddress, maxTimeout, lookback);
+ : getPrice(priceAggregatorAddress, maxTimeout)
}
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year 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.