Beanstalk Part 3
Beanstalk
DeFiHardhat
21,000 USDC
View results
Previous Next
Submission Details
Severity: high
Invalid

If `LibFertilizer.pop()` return false rewardToFertilizer is revert

hunter_w3b

Summary

Every Time LibFertilizer.pop() return false the rewardToFertilizer is revert due to require check.

if (!LibFertilizer.pop()) {
​
s.bpf = uint128(firstEndBpf); // 1002500
​
s.fertilizedIndex = s.fertilizedIndex.add(newFertilized);
​
require(s.fertilizedIndex == s.unfertilizedIndex, "Paid != owed");
return newFertilized;
}

Vulnerability Details

When fertilizer is added through the addFertilizer function, it sets specific state variables.

The following state variables are involved:

mapping(uint128 => uint256) fertilizer;
mapping(uint128 => uint128) nextFid;
uint256 activeFertilizer;
uint256 fertilizedIndex;
uint256 unfertilizedIndex;
uint128 fFirst;
uint128 fLast;
uint128 bpf;
uint256 recapitalized;
bool season.fertilizing;

  1. Add Fertilizer:

  • Calls the addFertilizer function with specific parameters.

  • Sets the fertilizer, activeFertilizer, unfertilizedIndex, fFirst, fLast, and season.fertilizing state variables.

I have Explain this bug by Manual analysis in simple paramaters:

step1: season = 6074 first season ,tokenAmountIn = 5e18,fertilizerAmount = 5, minLP = 2e18

function addFertilizer(
uint128 season, // 6074 first season
uint256 tokenAmountIn, // 5e18
uint256 fertilizerAmount, // 5
uint256 minLP // 2e18
) internal returns (uint128 id) {
AppStorage storage s = LibAppStorage.diamondStorage();
​
uint128 fertilizerAmount128 = fertilizerAmount.toUint128(); // @audit @>> 5
​
// @audit If we pass 6074 to getBpf
uint128 bpf = getBpf(season); // return value for getBpf @>> 1002500
​
// default_value(0) + 5 * 1002500 @>> 5012500
s.unfertilizedIndex = s.unfertilizedIndex.add(fertilizerAmount.mul(bpf)); // 5012500
​
// s.bpf = 0 default_value for only for this example
id = s.bpf.add(bpf); // @>> 1002500
​
// s.fertilizer[1002500] = 0 + 5; @>>5
s.fertilizer[id] = s.fertilizer[id].add(fertilizerAmount128);
​
​
s.activeFertilizer = s.activeFertilizer.add(fertilizerAmount); // @>>5
​
​
addUnderlying(tokenAmountIn, fertilizerAmount.mul(DECIMALS), minLP);
​
// 5 > 5 @@>> false
if (s.fertilizer[id] > fertilizerAmount128) return id;
​
push(id); // @@>> 1002500
emit SetFertilizer(id, bpf);
}
​
function push(uint128 id) internal {
AppStorage storage s = LibAppStorage.diamondStorage();
​
if (s.fFirst == 0) {
// Queue is empty
s.season.fertilizing = true;
s.fLast = id; // 1002500
s.fFirst = id; // 1002500
} else if (id <= s.fFirst) {
//.....
}

  • State Update in Step_1

    s.fertilizer[1002500] = 5;
    s.nextFid// not initialized yet
    s.fertilizedIndex;// zero for now
    s.unfertilizedIndex = 5012500;
    s.activeFertilizer = 5;
    s.fFirst = 1002500;
    s.fLast = 1002500;
    ​
    s.season.fertilizing = true;

  1. Reward Distribution --> sun.sol

deltaB = 50e6

function stepSun(int256 deltaB, uint256 caseId) internal {
if (deltaB > 0) {
uint256 newHarvestable = rewardBeans(uint256(deltaB));
setSoilAbovePeg(newHarvestable, caseId);
s.season.abovePeg = true;
}
​
// Below peg
else {
setSoilBelowPeg(deltaB);
s.season.abovePeg = false;
}
}

If the deltaB is greater then zero --> call the rewardBeans and pass the value of deltaB

function rewardBeans(uint256 newSupply) internal returns (uint256 newHarvestable) { // @audit this will return 1
​
uint256 newFertilized;
​
C.bean().mint(address(this), newSupply);
// @audit so our s.season.fertilizing is true because of we add Fertilizer in step_1
if (s.season.fertilizing) {
newFertilized = rewardToFertilizer(newSupply);
newSupply = newSupply.sub(newFertilized);
}

We called the rewardToFertilizer function because s.season.fertilizing is true

function rewardToFertilizer(uint256 amount) internalreturns (uint256 newFertilized) {
​
// @audit the value of amount = 50e6
uint256 maxNewFertilized = amount.div(FERTILIZER_DENOMINATOR); /// 50e6/3 --> 16666666
​
uint256 newBpf = maxNewFertilized.div(s.activeFertilizer); // 16666666 / 5 --> 3333333
​
uint256 oldTotalBpf = s.bpf; // 0
​
uint256 newTotalBpf = oldTotalBpf.add(newBpf); // 3333333
​
uint256 firstEndBpf = s.fFirst; // 1002500
​
// @audit if the value of newTotalBpf is bigger then firstEndBpf we enter to while loop
​
//3333333 >= 1002500
while(newTotalBpf >= firstEndBpf) {
​
// 1002500 - 0 --> 1002500
newBpf = firstEndBpf.sub(oldTotalBpf); //-----> 1002500
​
// 1002500 * 5 = 5012500
newFertilized = newFertilized.add(newBpf.mul(s.activeFertilizer)); // 5012500
​
if (!LibFertilizer.pop()) {
​
s.bpf = uint128(firstEndBpf); // 1002500
​
s.fertilizedIndex = s.fertilizedIndex.add(newFertilized);
​
require(s.fertilizedIndex == s.unfertilizedIndex, "Paid != owed");
return newFertilized;
}

we call the LibFertilizer.pop() and check if this return true or false

function pop() internal returns (bool) {
AppStorage storage s = LibAppStorage.diamondStorage();
​
​
uint128 first = s.fFirst; // 1002500
s.activeFertilizer = s.activeFertilizer.sub(getAmount(first)); // @@>> return s.fertilizer[1002500];
// 5 - 5 = 0 @audit we set the value of s.activeFertilizer to zero
​
uint128 next = getNext(first); // 1002500 @@>> s.nextFid[id]; return the zero value
if (next == 0) {
require(s.activeFertilizer == 0, "Still active fertilizer");
s.fFirst = 0;
s.fLast = 0;
s.season.fertilizing = false;
return false;
}
s.fFirst = getNext(first);
return true;
}
​

so we return false and the value of s.fFirst = 0, s.fLast = 0, s.season.fertilizing = false set back to the default values.

​
if (!LibFertilizer.pop()) {
s.bpf = uint128(firstEndBpf); // 1002500
// @audit the value of fertilizedIndex is zero we add @@>> 5012500
​
s.fertilizedIndex = s.fertilizedIndex.add(newFertilized); // 5012500
require(s.fertilizedIndex == s.unfertilizedIndex, "Paid != owed");
return newFertilized;
}
uint256 fertilizedIndex; // 5012500
uint256 unfertilizedIndex; // 5012500

the value of the fertlizeIndex and unfertilizedIndex is equal this is revert stepSun function

function stepSun(int256 deltaB, uint256 caseId) internal {
if (deltaB > 0) {
uint256 newHarvestable = rewardBeans(uint256(deltaB)); // return 1
setSoilAbovePeg(newHarvestable, caseId);
s.season.abovePeg = true;
}

so we can never s.season.abovePeg make to true.

Resulting State

After adding fertilizer and attempting to reward it, the following states are reached:

uint256 fertilizedIndex = 5012500;
uint256 unfertilizedIndex = 5012500;
bool season.fertilizing = false;
uint256 activeFertilizer = 0;

Impact

If the require(s.fertilizedIndex == s.unfertilizedIndex, "Paid != owed") statement fails, the transaction reverts. This revert prevents the season.abovePeg flag from being set.

  • No rewards being distributed for fertilizer.

  • Inability to set the season.abovePeg flag to true.

Tools Used

Manual Review

Recommendations

Sun::rewardToFertilizer remove the s.fertilizeIndex == s.unfertilizedIndex check

if (!LibFertilizer.pop()) {
s.bpf = uint128(firstEndBpf);
​
s.fertilizedIndex = s.fertilizedIndex.add(newFertilized);
- require(s.fertilizedIndex == s.unfertilizedIndex, "Paid != owed");
return newFertilized;
}
Updates

Lead Judging Commences

giovannidisiena Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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