Beanstalk: The Finale
Beanstalk
DeFiHardhatFoundry
250,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Wrong caseId set due to failing oracle

holydevoti0n

Summary

On the Sunrise process, there is the call to stepOracle. If one of the oracle fails when fetching the price, the storage for reserves/liquidity in USD is set to zero.

function updateOracle(
address well,
bytes memory lastSnapshot
) internal returns (int256 deltaB) {
AppStorage storage s = LibAppStorage.diamondStorage();
uint256[] memory twaReserves;
uint256[] memory ratios;
(deltaB, s.wellOracleSnapshots[well], twaReserves, ratios) = twaDeltaB(
well,
lastSnapshot
);
​
// Set the Well reserves in storage, so that it can be read when
// 1) set the USD price of the non bean token so that it can be read when
// calculating the price of Bean. See {LibEvaluate.evalPrice}.
// 2) When calculating the Bean reward for calling the Season (Bean:Eth Only).
// See {LibIncentive.determineReward}.
@> LibWell.setTwaReservesForWell(well, twaReserves); // @audit twaReserves empty
@> LibWell.setUsdTokenPriceForWell(well, ratios); // @audit ratios empty
​
emit WellOracle(
s.season.current,
well,
deltaB,
s.wellOracleSnapshots[well]
);
}

The problem here is that when calculating the caseId the function calcLPToSupplyRatiofrom LibEvalulatewill determine the lpToSupplyRatiobased using the totalUsdLiquiditywhich is the sum of the liquidity.

'liquidity' is definied as the non-bean value in a pool that trades beans.

But when one oracle or more fails, it will impact heavily the lpToSupplyRatiowhich is used to determine the value of the caseId.

function calcLPToSupplyRatio(
uint256 beanSupply
)
internal
view
returns (Decimal.D256 memory lpToSupplyRatio, address largestLiqWell, bool oracleFailure)
{
// prevent infinite L2SR
if (beanSupply == 0) return (Decimal.zero(), address(0), true);
​
​
address[] memory pools = LibWhitelistedTokens.getWhitelistedLpTokens();
uint256[] memory twaReserves;
uint256 totalUsdLiquidity;
uint256 largestLiq;
uint256 wellLiquidity;
for (uint256 i; i < pools.length; i++) {
// get the non-bean value in an LP.
twaReserves = LibWell.getTwaReservesFromStorageOrBeanstalkPump(pools[i]);
​
​
// calculate the non-bean usd liquidity value.
@> uint256 usdLiquidity = LibWell.getWellTwaUsdLiquidityFromReserves( // @audit oracle failed - 0
pools[i],
twaReserves
);
​
​
// if the usdLiquidty is 0, beanstalk assumes oracle failure.
if (usdLiquidity == 0) {
oracleFailure = true;
}
​
​
// calculate the scaled, non-bean liquidity in the pool.
wellLiquidity = getLiquidityWeight(pools[i]).mul(usdLiquidity).div(1e18); // @audit 0
​
​
// if the liquidity is the largest, update `largestLiqWell`,
// and add the liquidity to the total.
// `largestLiqWell` is only used to initialize `s.sopWell` upon a sop,
// but a hot storage load to skip the block below
// is significantly more expensive than performing the logic on every sunrise.
if (wellLiquidity > largestLiq) {
largestLiq = wellLiquidity;
largestLiqWell = pools[i];
}
​
​
@> totalUsdLiquidity = totalUsdLiquidity.add(wellLiquidity); // @audit 0
​
​
if (pools[i] == LibBarnRaise.getBarnRaiseWell()) {
// Scale down bean supply by the locked beans, if there is fertilizer to be paid off.
// Note: This statement is put into the for loop to prevent another extraneous read of
// the twaReserves from storage as `twaReserves` are already loaded into memory.
if (LibAppStorage.diamondStorage().sys.season.fertilizing == true) {
beanSupply = beanSupply.sub(LibUnripe.getLockedBeans(twaReserves));
}
}
​
​
// If a new non-Well LP is added, functionality to calculate the USD value of the
// liquidity should be added here.
}
​
​
// if there is no liquidity,
// return 0 to save gas.
if (totalUsdLiquidity == 0) return (Decimal.zero(), address(0), true);
​
​
// USD liquidity is scaled down from 1e18 to match Bean precision (1e6).
@> lpToSupplyRatio = Decimal.ratio(totalUsdLiquidity.div(LIQUIDITY_PRECISION), beanSupply);
}

Scenario: When calling Sunrise, the following occurs:

  • ✅ WETH/BEAN - $20k in WETH liquidity - oracle succeeds, therefore twaReserves and usdLiquidity are stored. Then those values are used to set the totalUsdLiquidityon calcLPToSupplyRatio.

  • ❌ WSTETH/BEAN - 500k in WSTETH liquidity - oracle failed, therefore twaReserves and usdLiquidityare zero.

The lpToSupplyRatiothat should have the value Decimal.ratio(520k.div(1e12), beanSupply); in case both oracles' succeed, will be miscalculated as:

Decimal.ratio(20k.div(1e12), beanSupply);

Notice that the beanSupplydoesn't change, so here we have a result that differs a lot from what is the current non-bean liquidity. Beanstalk will miscalculate the caseId:

  • Let's say that the correct lpToSupplyLiquidity should be greater than the lpToSupplyRatioUpperBound but as only the well with low liquidity has succeeded in fetching the price of the non-bean asset, the lpToSupplyLiquidity will be considered lpToSupplyRatioLowerBound

  • In a nutshell, the caseIdthat should be 108is now set to 36.

function evalLpToSupplyRatio(
Decimal.D256 memory lpToSupplyRatio
) internal view returns (uint256 caseId) {
AppStorage storage s = LibAppStorage.diamondStorage();
// Extremely High
if (
lpToSupplyRatio.greaterThanOrEqualTo(
s.sys.seedGaugeSettings.lpToSupplyRatioUpperBound.toDecimal()
)
) {
@> caseId = 108;
// Reasonably High
} else if (
lpToSupplyRatio.greaterThanOrEqualTo(
s.sys.seedGaugeSettings.lpToSupplyRatioOptimal.toDecimal()
)
) {
@> caseId = 72;
// Reasonably Low
} else if (
lpToSupplyRatio.greaterThanOrEqualTo(
s.sys.seedGaugeSettings.lpToSupplyRatioLowerBound.toDecimal()
)
) {
@> caseId = 36;
}
// excessively low (caseId = 0)
}

This is just one scenario, as there will be several wells with different liquidity, the caseIdwill be often at risk of being set with an inappropriate value.

See that the calculation of the caseIdis 100% dependent on evalLpToSupplyRatio

/**
@> * @notice Evaluates beanstalk based on deltaB, podRate, deltaPodDemand and lpToSupplyRatio.
@> * and returns the associated caseId.
*/
function evaluateBeanstalk(int256 deltaB, uint256 beanSupply) internal returns (uint256, bool) {
BeanstalkState memory bs = updateAndGetBeanstalkState(beanSupply);
uint256 caseId = evalPodRate(bs.podRate) // Evaluate Pod Rate
.add(evalPrice(deltaB, bs.largestLiqWell))
.add(evalDeltaPodDemand(bs.deltaPodDemand))
@> .add(evalLpToSupplyRatio(bs.lpToSupplyRatio)); // Evaluate Price // Evaluate Delta Soil Demand // Evaluate LP to Supply Ratio
return (caseId, bs.oracleFailure);
}

Also, there is an assumption on the updateTemperatureAndBeanToMaxLpGpPerBdvRatio function that the liquidity level doesn't affect the temperature.

/**
* @notice updates the temperature and BeanToMaxLpGpPerBdvRatio, based on the caseId.
* @param caseId the state beanstalk is in, based on the current season.
@> * @dev currently, an oracle failure does not affect the temperature, as
@> * the temperature is not affected by liquidity levels. The function will
@> * need to be updated if the temperature is affected by liquidity levels.
* This is implemented such that liveliness in change in temperature is retained.
*/
function updateTemperatureAndBeanToMaxLpGpPerBdvRatio(
uint256 caseId,
bool oracleFailure
) internal {
@> LibCases.CaseData memory cd = LibCases.decodeCaseData(caseId);
@> updateTemperature(cd.bT, caseId);
​
// if one of the oracles needed to calculate usd liquidity fails,
// the beanToMaxLpGpPerBdvRatio should not be updated.
if (oracleFailure) return;
updateBeanToMaxLPRatio(cd.bL, caseId);
}

See that the oracleFailuredoesn't prevent the temperature to be updated using the miscalculated caseId.

In the last line tagged on the NatSpec: The function will need to be updated if the temperature is affected by liquidity levels.

Because of this issue, the peg mechanism will frequently be affected, resulting in:

Impact

  • Miscalculation of the temperature, impacting on how Beanstalk issues debt/rewards.

  • Miscalculation for the Flood/Sop. If P > 1 and the * Pod Rate is less than 5%, the Farm is Oversaturated. If it is Oversaturated * for a Season, each Season in which it continues to be Oversaturated, it Floods.Impact

  • With all the above, the peg mechanism is compromised.

Tools Used

Manual Review

Recommendations

(Recommended) When an oracle fails, the protocol should adopt a conservative strategy when setting the caseId on evalLpToSupplyRatioto avoid unintended changes in the temperature.

(Additional) In one of my previous reports, I suggested the latest 'safe' price for a similar case, but I think we can use it here as well.

Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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