Beanstalk Part 2
Beanstalk
DeFiHardhat
35,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Mapping `nextFid` misbehaves when `push(id)` is called against business logic

WeBuiltDifferent

Summary

The addFertilizer() function in LibFertilizer.sol harbors a discrepancy deviating from its inline documentation. It consistently invokes push(id) without appropriate verification, leading to undesired outcomes within the nextFid mapping.

Vulnerability Details

The flaw manifests when push(id) is invoked for an existing id, thereby disrupting the integrity of the nextFid mapping. For instance, in a scenario where sequential mappings are as follows:

1 => 5
5 => 10
10 => 15
15 => 20

If an existing mapping other than the first or last, such as 10, is called, the mapping undergoes alterations:

1 => 5
5 => 5
10 => 15
15 => 20

Consequently, this breakdown in the chain could potentially induce bugs if utilized elsewhere in the codebase.

Impact

The aberrant behavior exhibited by the nextFid mapping might propagate misinformation if accessed for frontend purposes or induce computational anomalies if utilized off-chain.

Tools Used

Remix IDE was used to test this because of the absence of a foundry testing suite in the protocol.
The following code can be used to cross check.

//SPDX-License-Identifier:MIT
pragma solidity =0.7.6;
pragma abicoder v2;
contract push {
uint128 public fFirst;
uint128 public fLast;
bool public fertilizing;
mapping(uint128 => uint128) public nextFid;
function _push(uint128 id) public {
// if queue is empty, set season to true
if (fFirst == 0) {
// Queue is empty
fertilizing = true;
fLast = id;
fFirst = id;
} else if (id <= fFirst) {
// Add to front of queue
setNext(id, fFirst);
fFirst = id;
} else if (id >= fLast) {
// Add to back of queue
setNext(fLast, id);
fLast = id;
} else {
// Add to middle of queue
uint128 prev = fFirst;
uint128 next = getNext(prev);
// Search for proper place in line
// question what if id is exactly equal to something in the middle?
while (id > next) {
prev = next;
next = getNext(next);
}
setNext(prev, id);
setNext(id, next);
}
}
function getNext(uint128 id) internal view returns (uint128) {
// nextFid mapping of previous id to next id
return nextFid[id];
}
function setNext(uint128 id, uint128 next) internal {
// nextFid mapping of previous id to next id
nextFid[id] = next;
}
}

Recommendations

The following changes can be made

// If not first time adding Fertilizer with this id, return
if (s.fertilizer[id] > fertilizerAmount128) {
return id;
} else {
// If first time, log end Beans Per Fertilizer and add to Season queue.
push(id);
}
Updates

Lead Judging Commences

giovannidisiena Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

Informational/Invalid

Support

FAQs

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