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

`LibReceiving.barnReceive()` doesn't reset `s.sys.fert.leftoverBeans` when debt is fully repaid

T1MOH

Summary

Shipment planner introduces new concept of leftoverBeans. It is beans which are already minted to Fertilizer but not yet queued to be distributed. So any extra beans which are minted to Fertilizer at the start of the season are written to this variable.

Problem is that this variable isn't reset on the very last distribution of Beans to Fertilizer, i.e. when it finally repays debt. In this case positive value of variable leftoverBeans affects protocol in a way described in Impact section

Vulnerability Details

Here is the reference to code snippet with algorithm of Fertilizer distribution. Tbh it's difficult to explain by words, I suggest to study how it works to understand issue.
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/df2dd129a878d16d4adc75049179ac0029d9a96b/protocol/contracts/libraries/LibReceiving.sol#L103-L152

function barnReceive(uint256 shipmentAmount, bytes memory) private {
AppStorage storage s = LibAppStorage.diamondStorage();
​
@> uint256 amountToFertilize = shipmentAmount + s.sys.fert.leftoverBeans;
​
// Get the new Beans per Fertilizer and the total new Beans per Fertilizer
// Zeroness of activeFertilizer handled in Planner.
uint256 remainingBpf = amountToFertilize / s.sys.fert.activeFertilizer;
uint256 oldBpf = s.sys.fert.bpf;
uint256 newBpf = oldBpf + remainingBpf;
​
// Get the end BPF of the first Fertilizer to run out.
uint256 firstBpf = s.sys.fert.fertFirst;
​
uint256 deltaFertilized;
​
// If the next fertilizer is going to run out, then step BPF according
while (newBpf >= firstBpf) {
// Increment the cumulative change in Fertilized.
deltaFertilized += (firstBpf - oldBpf) * s.sys.fert.activeFertilizer; // fertilizer between init and next cliff
​
if (LibFertilizer.pop()) {
oldBpf = firstBpf;
firstBpf = s.sys.fert.fertFirst;
// Calculate BPF beyond the first Fertilizer edge.
remainingBpf = (amountToFertilize - deltaFertilized) / s.sys.fert.activeFertilizer;
newBpf = oldBpf + remainingBpf;
}
// Else, if there is no more fertilizer. Matches plan cap.
else {
s.sys.fert.bpf = uint128(firstBpf); // SafeCast unnecessary here.
s.sys.fert.fertilizedIndex += deltaFertilized;
require(amountToFertilize == deltaFertilized, "Inexact amount of Beans at Barn");
require(s.sys.fert.fertilizedIndex == s.sys.fert.unfertilizedIndex, "Paid != owed");
return;
}
}
​
// Distribute the rest of the Fertilized Beans
s.sys.fert.bpf = uint128(newBpf); // SafeCast unnecessary here.
deltaFertilized += (remainingBpf * s.sys.fert.activeFertilizer);
s.sys.fert.fertilizedIndex += deltaFertilized;
​
@> // There will be up to activeFertilizer Beans leftover Beans that are not fertilized.
@> // These leftovers will be applied on future Fertilizer receipts.
@> s.sys.fert.leftoverBeans = amountToFertilize - deltaFertilized;
​
// Confirm successful receipt.
emit Receipt(ShipmentRecipient.BARN, shipmentAmount, abi.encode(""));
}

Impact

In case penaltimate season before the final debt repayment had positive leftoverBeans, it will have 2 impacts:

  1. All subsequent Sunrizes cannot perform because of revert in main gm() function.

  2. fundsSafu modifier will halt whole protocol because invariant check can't pass.

First revert on Barn shipment cap calculation here because of underflow, because totalUnfertilizedBeans is 0 and leftoverBeans is greater than 0:
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/df2dd129a878d16d4adc75049179ac0029d9a96b/protocol/contracts/ecosystem/ShipmentPlanner.sol#L57

function getBarnPlan(bytes memory) external view returns (ShipmentPlan memory shipmentPlan) {
if (!beanstalk.isFertilizing()) return shipmentPlan;
return
ShipmentPlan({
points: BARN_POINTS,
@> cap: beanstalk.totalUnfertilizedBeans() - beanstalk.leftoverBeans()
});
}

Second is because it requires extra Beans balance of leftoverBeans in Beanstalk, which is wrong. And obviously Beanstalk will not have that balance:
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/df2dd129a878d16d4adc75049179ac0029d9a96b/protocol/contracts/beanstalk/Invariable.sol#L186

if (tokens[i] == C.BEAN) {
entitlements[i] +=
(s.sys.fert.fertilizedIndex -
s.sys.fert.fertilizedPaidIndex +
@> s.sys.fert.leftoverBeans) + // unrinsed rinsable beans
s.sys.silo.unripeSettings[C.UNRIPE_BEAN].balanceOfUnderlying; // unchopped underlying beans

Tools Used

Manual Review

Recommendations

Reset leftoverBeans when debt is fully repaid:

function barnReceive(uint256 shipmentAmount, bytes memory) private {
...
​
// If the next fertilizer is going to run out, then step BPF according
while (newBpf >= firstBpf) {
...
​
if (LibFertilizer.pop()) {
...
}
// Else, if there is no more fertilizer. Matches plan cap.
else {
s.sys.fert.bpf = uint128(firstBpf); // SafeCast unnecessary here.
s.sys.fert.fertilizedIndex += deltaFertilized;
+ s.sys.fert.leftoverBeans = 0;
require(amountToFertilize == deltaFertilized, "Inexact amount of Beans at Barn");
require(s.sys.fert.fertilizedIndex == s.sys.fert.unfertilizedIndex, "Paid != owed");
return;
}
}
​
...
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Validated
Assigned finding tags:

shipment barnReceive s.sys.fert.leftoverBeans missing state update

Appeal created

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

shipment barnReceive s.sys.fert.leftoverBeans missing state update

Support

FAQs

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