Beanstalk Part 1
Beanstalk
DeFiHardhatOracleProxyUpdates
100,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

Temperature and caseId are incorrectly adjusted when oracle fails

holydevoti0n

Summary

When user calls gm and the call for the chainlink oracle fails, it will return 0 for the deltaB value and this will cause a cascade effect, making the calculation of caseId = 3 and using the incorrect caseId to set up the new temperature on Weather.sol

function updateTemperature(int8 bT, uint256 caseId) private {
uint256 t = s.w.t;
if (bT < 0) {
if (t <= uint256(-bT)) {
// if (change < 0 && t <= uint32(-change)),
// then 0 <= t <= type(int8).max because change is an int8.
// Thus, downcasting t to an int8 will not cause overflow.
bT = 1 - int8(t);
s.w.t = 1;
} else {
s.w.t = uint32(t - uint256(-bT));
}
} else {
s.w.t = uint32(t + uint256(bT));
}
​
emit TemperatureChange(s.season.current, caseId, bT);
}

Every consumer of the temperature on the protocol will be affected like:

  • LibDibbler.morningTemperature

  • LibDibbler.beansToPods

  • LibDibbler.remainingPods

  • Sun.setSoilAbovePeg

  • Sun.stepSun

  • FieldFacet.maxTemperature

  • FieldFacet.totalSoil

  • FieldFacet._totalSoilAndTemperature

  • `FieldFacet.sowWithMin

Vulnerability Details

gm function uses the incorrect deltaB(0) to calculate the caseId which is then used to set the temperature.

function gm(address account, LibTransfer.To mode) public payable returns (uint256) {
int256 deltaB = stepOracle(); // @audit here if oracle failed, we update the season.timestamp and return deltaB zero here
uint256 caseId = calcCaseIdandUpdate(deltaB); // @audit caseId will be 3 here if deltaB is zero
LibGerminate.endTotalGermination(season, LibWhitelistedTokens.getWhitelistedTokens());
LibGauge.stepGauge();
stepSun(deltaB, caseId); // @audit wrong deltaB and caseId used here, setting the abovePeg to false and soil to zero
}

Impact

The interest rate will be wrongly decreased to 1, compromising the protocol peg mechanism when it needs to be maintained with a high interest rate/ temperature.

Sow will be calculated with the lowest temperature, also compromising the peg mechanism due to the wrong exchange of Beans -> Sow -> Pods

Remaining pods function will return zero and users will have an inaccurate number representing their actual pods.

PoC

  1. Prepare the environment to work with Foundry + Updated Mocks
    https://gist.github.com/h0lydev/fcdb00c797adfdf8e4816031e095fd6c

  2. Make sure to have the mainnet forked through Anvil: anvil --fork-url https://rpc.ankr.com/eth

  3. Create the SeasonTemperature.t.sol file under the folder foundry and paste the code below. Then run forge test --match-contract SeasonTemperatureTest -vv.

// SPDX-License-Identifier: MIT
pragma solidity =0.7.6;
pragma abicoder v2;
​
import { Sun } from "contracts/beanstalk/sun/SeasonFacet/Sun.sol";
import { MockSeasonFacet } from "contracts/mocks/mockFacets/MockSeasonFacet.sol";
import { MockSiloFacet } from "contracts/mocks/mockFacets/MockSiloFacet.sol";
import { MockFieldFacet } from "contracts/mocks/mockFacets/MockFieldFacet.sol";
import { MockWhitelistFacet } from "contracts/mocks/mockFacets/MockWhitelistFacet.sol";
import {LibWhitelist} from "contracts/libraries/Silo/LibWhitelist.sol";
import { Utils } from "./utils/Utils.sol";
import "./utils/TestHelper.sol";
import "contracts/libraries/LibSafeMath32.sol";
import "contracts/C.sol";
​
contract SeasonTemperatureTest is MockSeasonFacet, TestHelper {
using SafeMath for uint256;
using LibSafeMath32 for uint32;
​
bool oracleFailed;
function setUp() public {
console.log("diamondSetup");
vm.createSelectFork('local');
oracleFailed = false;
setupDiamond();
dewhitelistCurvePool();
mintUnripeLPToUser1();
mintUnripeBeanToUser1();
setOraclePrices(false, 1000e6, 1000e6, 1000e6);
_setReservesForWell(1000000e6, 1000e18);
// user / tokens
mintTokenForUsers();
setTokenApprovalForUsers();
enableFertilizerAndMintActiveFertilizers();
​
callSunriseForUser1();
}
​
​
//////////// Setup functions ////////////
​
function setTokenApprovalForUsers() internal {
approveTokensForUser(deployer);
approveTokensForUser(user1);
approveTokensForUser(user2);
approveTokensForUser(user3);
approveTokensForUser(user4);
approveTokensForUser(user5);
}
​
function mintTokenForUsers() internal {
mintWETHtoUser(deployer);
mintWETHtoUser(user1);
mintWETHtoUser(user2);
mintWETHtoUser(user3);
mintWETHtoUser(user4);
mintWETHtoUser(user5);
​
// mint C.bean() to users
C.bean().mint(deployer, 10e6);
C.bean().mint(user1, 10e6);
C.bean().mint(user2, 10e6);
C.bean().mint(user3, 10e6);
C.bean().mint(user4, 10e6);
C.bean().mint(user5, 10e6);
}
​
function approveTokensForUser(address user) prank(user) internal {
mockWETH.approve(address(diamond), type(uint256).max);
unripeLP.approve(address(diamond), type(uint256).max);
unripeBean.approve(address(diamond), type(uint256).max);
well.approve(address(diamond), type(uint256).max);
C.bean().approve(address(field), type(uint256).max);
C.bean().approve(address(field), type(uint256).max);
}
​
function dewhitelistCurvePool() public {
vm.prank(deployer);
whitelist.dewhitelistToken(C.CURVE_BEAN_METAPOOL);
}
​
function mintWETHtoUser(address user) prank(user) internal {
mockWETH.mint(user, 1000e18);
}
​
function mintUnripeLPToUser1() internal {
unripeLP.mint(user1, 1000e6);
}
​
function mintUnripeBeanToUser1() internal {
unripeBean.mint(user1, 1000e6);
}
​
function enableFertilizerAndMintActiveFertilizers() internal {
// second parameter is the unfertilizedIndex
fertilizer.setFertilizerE(true, 10000e6);
​
vm.prank(deployer);
fertilizer.addFertilizerOwner(7500, 1e11, 99);
​
vm.prank(deployer);
fertilizer.addFertilizerOwner(6200, 1e11, 99);
​
addUnripeTokensToFacet();
}
​
function addUnripeTokensToFacet() prank(deployer) internal {
unripe.addUnripeToken(C.UNRIPE_BEAN, C.BEAN, bytes32(0));
unripe.addUnripeToken(C.UNRIPE_LP, C.BEAN_ETH_WELL, bytes32(0));
}
​
function callSunriseForUser1() prank(user1) internal {
_ensurePreConditions();
_advanceInTime(2 hours);
season.sunrise();
}
​
function setOraclePrices(bool makeOracleFail, int256 chainlinkPrice, uint256 ethUsdtPrice, uint256 ethUsdcPrice) internal {
if (makeOracleFail) {
_addEthUsdPriceChainlink(0);
oracleFailed = true;
} else {
oracleFailed = false;
_addEthUsdPriceChainlink(chainlinkPrice);
_setEthUsdtPrice(ethUsdtPrice);
_setEthUsdcPrice(ethUsdcPrice);
}
}
​
////////////////////////////////////////// TESTS //////////////////////////////////////////
​
function testWrongCalcId_whenOracleFails() public {
_prepareForAbovePeg();
_advanceInTime(1 hours);
uint256 _snapId = vm.snapshot();
// When sunrise succeeds
vm.prank(user4);
season.sunrise();
​
// Then print results
_printProtocolState();
assertEq(season.getT(), 5, "when succeeds t should be 5");
// Then revert it to prepare for the season that will fail
vm.revertTo(_snapId);
​
// Prepare for the season that will fail
setOraclePrices(true, 0, 0, 0);
​
// When sunrise fails
vm.prank(user4);
season.sunrise();
​
console.log("Oracle failed, see results");
_printProtocolState();
assertEq(season.getT(), 1, "when succeeds t should be 1");
​
}
​
function _printProtocolState() internal {
console.log("-------------- Results --------------");
console.log("");
console.log("thisSowTime: ", season.thisSowTime());
console.log("lastSowTime: ", season.lastSowTime());
console.log("getUsdTokenPrice: ", season.getUsdTokenPrice());
console.log("getReserve0: ", season.getReserve0());
console.log("getReserve1: ", season.getReserve1());
console.log("getAbovePeg: ", season.getAbovePeg());
console.log("getSoil: ", season.getSoil());
console.log("lastDSoil: ", season.lastDSoil());
console.log("s.w.t: ", season.getT());
console.log("remaining pods: ", field.remainingPods());
}
​
function _prepareForAbovePeg() internal {
season.mockSetSopWell(address(well));
season.captureWellE(address(well));
season.setYieldE(5); // s.w.t
setOraclePrices(false, 1000e6, 1000e6, 1000e6);
​
season.setLastSowTimeE(1);
season.setNextSowTimeE(10);
season.calcCaseIdE(1e18, 1);
season.setAbovePegE(true);
}
​
////////////////////////////////////////// HELPERS //////////////////////////////////////////
​
function _ensurePreConditions() internal {
assertTrue(season.thisSowTime() == type(uint32).max, "thisSowTime should be max");
assertTrue(season.lastSowTime() == type(uint32).max, "thisLastSowTime should be max");
assertEq(season.getIsFarm(), 1, "isFarm should be 1");
assertEq(season.getUsdTokenPrice(), 1, "usdTokenPrice should be 1");
assertEq(season.getReserve0(), 1, "reserve0 should be 1");
assertEq(season.getReserve1(), 1, "reserve1 should be 1");
assertFalse(season.getAbovePeg(), "pre - abovePeg should be false");
assertEq(season.getSoil(), 0, "soil should be == 0");
}
}

Output:

handleRain caseId: 0
-------------- Results --------------
thisSowTime: 4294967295
lastSowTime: 4294967295
getUsdTokenPrice: 1
getReserve0: 1
getReserve1: 1
getAbovePeg: false
getSoil: 462832752243
lastDSoil: 0
s.w.t: 5
remaining pods: 467461079765
​
handleRain caseId: 3
Oracle failed, see results
-------------- Results --------------
thisSowTime: 4294967295
lastSowTime: 4294967295
getUsdTokenPrice: 1
getReserve0: 1
getReserve1: 1
getAbovePeg: false
getSoil: 0
lastDSoil: 0
s.w.t: 1
remaining pods: 0
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 29.45s (3.32ms CPU time)

ps: a console.log was added to the handleRain function to print the caseId.

Result: In a normal scenario the temperature would have remained at the value 5 but in this case was set to 1 and remaining pods/soil are also set to zero when in fact they should not.

Tools Used

Manual Review & Foundry

Recommendations

It is noticed that the developers have the intention of never reverting the sunrise function to decrease the risk of depeg and breaking the incentive for users calling it. But at the same time, those state variables shouldn't be updated as if the system is working correctly because they will impact the next season as stated in this finding.

It is tricky to propose a simple fix to the problem without impacting the system as a whole. Here are a few ideas that could be used:

  1. (Recommended) An effective solution could be store the latest response from chainlink and in case it fails and the timeout(a limit that you can be added to accept a previous response from the oracle) is not reached yet, protocol could use the previous response. Liquity Protocol uses this approach, an example here: https://github.com/liquity/dev/blob/main/packages/contracts/contracts/PriceFeed.sol
    This solution will be effective for the protocol because the oracle is also called in different places like when minting fertilizers(getMintFertilizerOut), getting the well price(getRatiosAndBeanIndex), and getConstantProductWell. As the oracle is used along the protocol in many places, the latest successful price would be often up-to-date and within the limit time defined to use the previous price when the chainlink oracle fails.

  • Additionally, consider handling the errors properly before updating the deltaB and abovePeg variables, as these disrupt the peg mechanism logic.

Updates

Lead Judging Commences

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

Incorrect case id oracle failure

Support

FAQs

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