DeFiHardhatFoundry
250,000 USDC
View results
Submission Details
Severity: low
Invalid

The sun reseed lacks the migration of the `s.sys.season.start` state variable

Summary

The sun reseed lacks the migration of the s.sys.season.start state variable. That could lead to executing several seasons in the same transaction and bypassing some flashloan protections

Relevant GitHub Links:

https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/beanstalk/init/reseed/L2/ReseedSun.sol#L36-L56

Vulnerability Details

The migration of the sun state is executed as follows:

function init(
uint32 season,
uint32 temperature,
uint128 averageGrownStalkPerBdvPerSeason,
uint128 beanToMaxLpGpPerBdvRatio
) external {
// @audit-issue s.sys.season.start is missing and could lead to several sunrise function calls being executable
s.sys.season.current = season;
s.sys.season.period = PERIOD;
s.sys.season.timestamp = TIMESTAMP;
s.sys.weather.temp = temperature;
s.sys.seedGauge.averageGrownStalkPerBdvPerSeason = averageGrownStalkPerBdvPerSeason;
emit BeanToMaxLpGpPerBdvRatioChange(
s.sys.season.current,
type(uint256).max,
int80(int128(beanToMaxLpGpPerBdvRatio))
);
s.sys.seedGauge.beanToMaxLpGpPerBdvRatio = beanToMaxLpGpPerBdvRatio;
emit UpdateAverageStalkPerBdvPerSeason(averageGrownStalkPerBdvPerSeason);
LibCases.setCasesV2();
}

We can see that only the current season number, the period, the timestamp and the temperature will be set during initialization. However, here lacks a really important state variable, the s.sys.season.start. This variable is used to check if enough time elapsed to call a new sunrise function. The function that checks it is the following:

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.");
...
}
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);
}

Let's take a real example with data that could happen in a real case. At the time of writing this report the L1 beanstalk contract has the following state variables corresponding to the sun:

{
"current": 22896, // season number
"start": "1637935200", // timestamp when the sun started
"period": "3600", // the amount of time required between season
"timestamp": "1720360811" // last sunrise call timestamp
}

Imagine that the current timestamp is 1720360820. In this specific moment and with this data it would not be possible to call a sunrise function in the L1 because:

seasonTime = (1720360820 - 1637935200) / 3600 = 22896

And 22896 is not greater than s.sys.season.current which is 22896. However, let's now evaluate if it would be possible to call the sunrise function on L2 when the reseed function would be called and s.sys.season.start would not be set:

seasonTime = (1720360820 - 0) / 3600 = 477878

That means that the sunrise function would be executable until reaching this season number. This is also possible because each sunrise function only advances a single season number regardless of the time elapsed and the condition above would still be true until reaching the season number 477878:

function stepSeason() private returns (uint32 season) {
s.sys.season.current += 1;
season = s.sys.season.current;
s.sys.season.sunriseBlock = uint32(block.number); // Note: Will overflow in the year 3650.
emit Sunrise(season);
}

This can be really dangerous because there might be some components of the protocol that rely on the season number. For example, when depositing into the silo, there is some stalk that needs to be germinated during 2 seasons, that could be exploited by somebody that takes a flashloan with a big amount of a token, deposits it, advances 2 seasons because of this issue, and he would have at the same transaction the stalk proportional to his huge deposit. All of this in a single transaction.

Impact

High, this issue can open some high risk exploits for the system

Tools Used

Manual review

Recommendations

Add the migration of the s.sys.season.start state variable from the L1:

function init(
uint32 season,
uint32 temperature,
+ uint32 start,
uint128 averageGrownStalkPerBdvPerSeason,
uint128 beanToMaxLpGpPerBdvRatio
) external {
s.sys.season.current = season;
+ s.sys.season.start = start;
s.sys.season.period = PERIOD;
s.sys.season.timestamp = TIMESTAMP;
s.sys.weather.temp = temperature;
s.sys.seedGauge.averageGrownStalkPerBdvPerSeason = averageGrownStalkPerBdvPerSeason;
emit BeanToMaxLpGpPerBdvRatioChange(
s.sys.season.current,
type(uint256).max,
int80(int128(beanToMaxLpGpPerBdvRatio))
);
s.sys.seedGauge.beanToMaxLpGpPerBdvRatio = beanToMaxLpGpPerBdvRatio;
emit UpdateAverageStalkPerBdvPerSeason(averageGrownStalkPerBdvPerSeason);
LibCases.setCasesV2();
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

The sun reseed lacks the migration of the `s.sys.season.start` state variable

Support

FAQs

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