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

Valid prices would get finalized as invalid due to non-inclusivity of `getWstethEthPrice()`'a price difference check

bauchibred

Summary

The getWstethEthPrice function is designed to fetch the price of wstEth by comparing prices from Chainlink and Uniswap oracles. However, it contains a check for price discrepancy (MAX_DIFFERENCE) and, if not met, returns a price of 0. The issue explained in this report arises because the check uses a strict < operator instead of <= when comparing the percent difference, leading to a failure in cases where the difference is exactly equal to MAX_DIFFERENCE. This could occur frequently in real-world market conditions, causing the function to return 0 for valid price queries, since it now assumes the returned price has been manipulated, leading to a denial of service (DOS) for protocol's pricing logic.

Vulnerability Details

Take a look at https://github.com/Cyfrin/2024-04-beanstalk-2/blob/27ff8c87c9164c1fbff054be5f22e56f86cdf127/protocol/contracts/libraries/Oracle/LibWstethEthOracle.sol#L69-L100

function getWstethEthPrice(uint256 lookback) internal view returns (uint256 wstethEthPrice) {
​
uint256 chainlinkPrice = lookback == 0 ?
LibChainlinkOracle.getPrice(WSTETH_ETH_CHAINLINK_PRICE_AGGREGATOR, LibChainlinkOracle.FOUR_DAY_TIMEOUT) :
LibChainlinkOracle.getTwap(WSTETH_ETH_CHAINLINK_PRICE_AGGREGATOR, LibChainlinkOracle.FOUR_DAY_TIMEOUT, lookback);
​
// Check if the chainlink price is broken or frozen.
if (chainlinkPrice == 0) return 0;
​
uint256 stethPerWsteth = IWsteth(C.WSTETH).stEthPerToken();
​
chainlinkPrice = chainlinkPrice.mul(stethPerWsteth).div(CHAINLINK_DENOMINATOR);
​
​
// Uniswap V3 only supports a uint32 lookback.
if (lookback > type(uint32).max) return 0;
uint256 uniswapPrice = LibUniswapOracle.getTwap(
lookback == 0 ? LibUniswapOracle.FIFTEEN_MINUTES :
uint32(lookback),
WSTETH_ETH_UNIV3_01_POOL, C.WSTETH, C.WETH, ONE
);
​
// Check if the uniswapPrice oracle fails.
if (uniswapPrice == 0) return 0;
//@audit when the difference is == `MAX_DIFFERENCE` would wrongly return zero
​
if (LibOracleHelpers.getPercentDifference(chainlinkPrice, uniswapPrice) < MAX_DIFFERENCE) {
wstethEthPrice = chainlinkPrice.add(uniswapPrice).div(AVERAGE_DENOMINATOR);
if (wstethEthPrice > stethPerWsteth) wstethEthPrice = stethPerWsteth;
wstethEthPrice = wstethEthPrice.div(PRECISION_DENOMINATOR);
}
}

This function is used to get the wstEth price, and with the strict MAX_DIFFERENCE check applied here, in cases where the price discrepancy is equal to the accepted MAX_DIFFERENCE this function returns 0, cause the check isn't <= keep in mind that the MAX_DIFFERENCE value is relatively minute and this would easily happen in real life market conditions.

Impact

Acceptable prices would be finalized as unacceptable causing an unintended return of 0 for price queries since protocol now assumes the price has been manipulated, essentially leading to an unintended DOS to LibWstethEthOracle.sol's getWstethEthPrice() and all instances it''s been used in protocol.

Tool Used

Manual review

Recommended Mitigation Steps

Consider applying this change to https://github.com/Cyfrin/2024-04-beanstalk-2/blob/27ff8c87c9164c1fbff054be5f22e56f86cdf127/protocol/contracts/libraries/Oracle/LibWstethEthOracle.sol#L69-L100

function getWstethEthPrice(uint256 lookback) internal view returns (uint256 wstethEthPrice) {
​
uint256 chainlinkPrice = lookback == 0 ?
LibChainlinkOracle.getPrice(WSTETH_ETH_CHAINLINK_PRICE_AGGREGATOR, LibChainlinkOracle.FOUR_DAY_TIMEOUT) :
LibChainlinkOracle.getTwap(WSTETH_ETH_CHAINLINK_PRICE_AGGREGATOR, LibChainlinkOracle.FOUR_DAY_TIMEOUT, lookback);
​
// Check if the chainlink price is broken or frozen.
if (chainlinkPrice == 0) return 0;
​
uint256 stethPerWsteth = IWsteth(C.WSTETH).stEthPerToken();
​
chainlinkPrice = chainlinkPrice.mul(stethPerWsteth).div(CHAINLINK_DENOMINATOR);
​
​
// Uniswap V3 only supports a uint32 lookback.
if (lookback > type(uint32).max) return 0;
uint256 uniswapPrice = LibUniswapOracle.getTwap(
lookback == 0 ? LibUniswapOracle.FIFTEEN_MINUTES :
uint32(lookback),
WSTETH_ETH_UNIV3_01_POOL, C.WSTETH, C.WETH, ONE
);
​
// Check if the uniswapPrice oracle fails.
if (uniswapPrice == 0) return 0;
​
- if (LibOracleHelpers.getPercentDifference(chainlinkPrice, uniswapPrice) < MAX_DIFFERENCE) {
+ if (LibOracleHelpers.getPercentDifference(chainlinkPrice, uniswapPrice) <= MAX_DIFFERENCE) {
wstethEthPrice = chainlinkPrice.add(uniswapPrice).div(AVERAGE_DENOMINATOR);
if (wstethEthPrice > stethPerWsteth) wstethEthPrice = stethPerWsteth;
wstethEthPrice = wstethEthPrice.div(PRECISION_DENOMINATOR);
}
}
Updates

Lead Judging Commences

giovannidisiena Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

wstETH:ETH price max difference

Support

FAQs

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