Weather Manual Review Findings

Weather Manual Review Findings

WEA-01M: Spot Reserve Season-of-Plenty Evaluation

Type Severity Location
Logical Fault Weather.sol:L179

Description:

The contract state that involves the stepWeather -> handleRain -> sop -> calculateSopBeansAndEth call sequence can be manipulated via flash loans as the amount of newBeans that should be used for the Season-o'-Plenty entirely relies on spot reserve evaluations of the peg and base pair of Beanstalk.

Example:

protocol/contracts/farm/facets/SeasonFacet/Weather.sol
175function calculateSopBeansAndEth() private view returns (uint256, uint256) {
176 (uint256 ethBeanPool, uint256 beansBeanPool) = reserves();
177 (uint256 ethUSDCPool, uint256 usdcUSDCPool) = pegReserves();
178
179 uint256 newBeans = sqrt(ethBeanPool.mul(beansBeanPool).mul(usdcUSDCPool).div(ethUSDCPool));
180 if (newBeans <= beansBeanPool) return (0,0);
181 uint256 beans = newBeans.sub(beansBeanPool);
182 beans = beans.mul(10000).div(9985).add(1);
183
184 uint256 beansWithFee = beans.mul(997);
185 uint256 numerator = beansWithFee.mul(ethBeanPool);
186 uint256 denominator = beansBeanPool.mul(1000).add(beansWithFee);
187 uint256 eth = numerator / denominator;
188
189 return (beans, eth);
190}

Recommendation:

We advise this trait of the system to be carefully evaluated as a malicious actor can manipulate the reserves before invoking the sunrise function of the SeasonFacet to achieve their desired SoP outcome.

Alleviation:

The Beanstalk team stated that any flash loan manipulating the peg of Beanstalk will cause a negative outcome for the potential attacker as the system's purpose is to normalize the peg at their expense or at the system's benefit. In light of these facts, we consider the exhibit adequately responded to and thus nullified as it is deemed as acceptable system behaviour.

WEA-02M: Unsafe Down-Casting

Type Severity Location
Mathematical Operations Weather.sol:L115

Description:

The linked down-casting to a uint96 is performed unsafely.

Example:

protocol/contracts/farm/facets/SeasonFacet/Weather.sol
115s.w.lastSoilPercent = uint96(endSoil.mul(1e18).div(bean().totalSupply()));

Recommendation:

We advise the down-casting to be performed safely as it can truncate in case the evaluation endSoil.mul(1e18).div(bean().totalSupply()) exceeds the maximum of a uint96 which has up to ~7.922e28 precision.

Alleviation:

The down-casting operation is now performed safely by yielding the maximum of uint96 if safe casting is impossible.

View Fix on GitHub

WEA-03M: Potentially Incorrect Edge Case

Type Severity Location
Logical Fault Weather.sol:L85, L90

Description:

In case the podRate is exactly equal to the optimal pod rate, it is possible for the caseId to be initialized to 16 and also offset by 4, conditions that should potentially be exclusive.

Example:

protocol/contracts/farm/facets/SeasonFacet/Weather.sol
83uint8 caseId = 0;
84if (podRate.greaterThanOrEqualTo(C.getUpperBoundPodRate())) caseId = 24;
85else if (podRate.greaterThanOrEqualTo(C.getOptimalPodRate())) caseId = 16;
86else if (podRate.greaterThanOrEqualTo(C.getLowerBoundPodRate())) caseId = 8;
87
88if (
89 int_price > 1e18 || (int_price == 1e18 &&
90 podRate.lessThanOrEqualTo(C.getOptimalPodRate()))
91) {
92 caseId += 4;
93}

Recommendation:

We advise the latter comparison to be set to a strict lessThan comparison to ensure non-overlapping conditions. The whitepaper is unclear as to the equality edge case of Rd being equal to Rd* and as such we advise the Beanstalk team to update the whitepaper correspondingly if this is desried behaviour.

Alleviation:

The latter comparison was properly set as non-inclusive (lessThan) to avoid overlapping conditions.

View Fix on GitHub

WEA-04M: Dynamic Evaluation of Supply

Type Severity Location
Logical Fault Weather.sol:L72, L115

Description:

The stepWeather function and its internal handleExtremeWeather function rely on a dynamic evaluation of bean().totalSupply() which can be significantly influenced via flash-loans that empty the trading pair as necessary and burn a large amount of BEAN units via the Beanstalk mechanisms.

Example:

protocol/contracts/farm/facets/SeasonFacet/Weather.sol
63function stepWeather(uint256 int_price, uint256 endSoil) internal {
64
65 if (bean().totalSupply() == 0) {
66 s.w.yield = 1;
67 return;
68 }
69
70 Decimal.D256 memory podRate = Decimal.ratio(
71 s.f.pods.sub(s.f.harvestable),
72 bean().totalSupply()
73 );
74
75 uint256 dsoil = s.w.startSoil.sub(endSoil);
76
77 Decimal.D256 memory deltaPodDemand;
78 uint256 lastDSoil = s.w.lastDSoil;
79 if (dsoil == 0) deltaPodDemand = Decimal.zero();
80 else if (lastDSoil == 0) deltaPodDemand = Decimal.from(1e18);
81 else deltaPodDemand = Decimal.ratio(dsoil, lastDSoil);
82
83 uint8 caseId = 0;
84 if (podRate.greaterThanOrEqualTo(C.getUpperBoundPodRate())) caseId = 24;
85 else if (podRate.greaterThanOrEqualTo(C.getOptimalPodRate())) caseId = 16;
86 else if (podRate.greaterThanOrEqualTo(C.getLowerBoundPodRate())) caseId = 8;
87
88 if (
89 int_price > 1e18 || (int_price == 1e18 &&
90 podRate.lessThanOrEqualTo(C.getOptimalPodRate()))
91 ) {
92 caseId += 4;
93 }
94
95 if (deltaPodDemand.greaterThanOrEqualTo(C.getUpperBoundDPD())) {
96 caseId += 2;
97 } else if (deltaPodDemand.greaterThanOrEqualTo(C.getLowerBoundDPD())) {
98 if (s.w.lastSowTime == MAX_UINT32 || !s.w.didSowBelowMin) {
99 caseId += 1;
100 }
101 else if (s.w.didSowFaster) {
102 caseId += 2;
103 s.w.didSowFaster = false;
104 }
105 }
106 s.w.lastDSoil = dsoil;
107 handleExtremeWeather(endSoil);
108 changeWeather(caseId);
109 handleRain(caseId);
110}
111
112function handleExtremeWeather(uint256 endSoil) private {
113 if (s.w.didSowBelowMin) {
114 s.w.didSowBelowMin = false;
115 s.w.lastSoilPercent = uint96(endSoil.mul(1e18).div(bean().totalSupply()));
116 s.w.lastSowTime = s.w.nextSowTime;
117 s.w.nextSowTime = MAX_UINT32;
118 }
119 else if (s.w.lastSowTime != MAX_UINT32) {
120 s.w.lastSowTime = MAX_UINT32;
121 }
122}

Recommendation:

We advise this trait of the system to be evaluated as it can cause the contract to artificially inflate the podRate in case of sharp adjustments. As we were not able to identify a profitable attack vector, this finding has been set to informational for consideration by the Beanstalk team.

Alleviation:

The Beanstalk team considered this exhibit and identified it as desirable behaviour by the system as the total supply needs to be utillized for the pod rate and an artificially inflated pod rate would not affect the integrity of the system given that there is a hard cap on how many beans can be burned for pods. As a result, we consider this exhibit nullified.

WEA-05M: Misleading Type Casting

Type Severity Location
Mathematical Operations Weather.sol:L128

Description:

The linked type casting of the yield value can become dangerous if the yield continuously increases due to fringe circumstances and then becomes reset.

Example:

protocol/contracts/farm/facets/SeasonFacet/Weather.sol
124function changeWeather(uint256 caseId) private {
125 int8 change = s.cases[caseId];
126 if (change < 0) {
127 if (yield() <= (uint32(-change))) {
128 change = 1 - int8(yield());
129 s.w.yield = 1;
130 }
131 else s.w.yield = yield()-(uint32(-change));
132 }
133 else s.w.yield = yield()+(uint32(change));
134
135 emit WeatherChange(season(), caseId, change);
136}

Recommendation:

We advise the change value to be set to a greater-accuracy value for the WeatherChange event as it can currently cause the change value to be misleading in the event's emission. Given that only an event is affected, the severity of this finding has been set to informational.

Alleviation:

The Beanstalk team stated that a casting underflow here is impossible based on the conditionals that surround it and have introduced comments to the code to indicate this. As a result, we consider this exhibit nullified.

Navigated to Weather Manual Review Findings