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

## Duplicate Entries in plotIndexes via `FieldFacet.sow`

ControlZ

Bean bug report

Duplicate Entries in plotIndexes via FieldFacet.sow

Description

The plotIndexes array is designed to contain unique plot indexes, ensuring that no index appears more than once. However, due to an oversight in the sow function in the FieldFacet, it is possible for multiple entries to have the same index.

Root Cause

During the sow function call, the index is assigned as follows:

// LibDibbler.sol
function sow(
// ...
) internal returns (uint256) {
// ...
uint256 index = s.sys.fields[s.sys.activeField].pods;
​
s.accts[account].fields[s.sys.activeField].plots[index] = pods;
s.accts[account].fields[s.sys.activeField].plotIndexes.push(index); // here the index is pushed
emit Sow(account, s.sys.activeField, index, beans, pods);
​
s.sys.fields[s.sys.activeField].pods += pods;
// ...
}

Here, the index is determined by s.sys.fields[s.sys.activeField].pods and is later incremented by the newly sown pods. However, if pods = 0, s.sys.fields[s.sys.activeField].pods remains unchanged, causing the same index to be pushed into the plotIndexes array on subsequent sow calls.

Impact

Functions utilizing the plotIndexes array will return incorrect values.
For example, getPlotsFromAccount will return the same plot multiple times.

Additionally, removePlotIndexFromAccount will only remove a single instance of the index, failing to perform correctly.

Proposed fix

Include a requirement in the sow function to ensure that pods > 0:

require(pods > 0, "Pods must be greater than 0");

Proof of Concept

To reproduce the issue, add the following test to protocol/test/foundry/field/Field.t.sol and execute it using forge test --match-test test_bug:

function test_bug(uint256 soil) public {
soil = bound(soil, 100, type(uint128).max);
​
C.bean().mint(farmers[0], soil);
​
_beforeEachSow(soil, soil / 2, false ? 1 : 0);
vm.prank(farmers[0]);
field.sow(0, 0, LibTransfer.From.EXTERNAL); // you can repeat this as much as wanted
vm.prank(farmers[0]);
field.sow(soil / 2, 0, LibTransfer.From.EXTERNAL);
​
uint256 activeField = field.activeField();
uint256[] memory plotIndexes = field.getPlotIndexesFromAccount(farmers[0], activeField);
MockFieldFacet.Plot[] memory plots = field.getPlotsFromAccount(farmers[0], activeField);
​
assertEq(plotIndexes.length, 3);
assertEq(plotIndexes[1], plotIndexes[2]); // altough plot indexes sould be unique
​
uint256 sumOfPlots = 0;
for (uint256 i = 0; i < plots.length; ++i) {
sumOfPlots += plots[i].pods;
}
assertGt(sumOfPlots, soil); // plots[1] and plots[2] are both pointing to the same plot!
}
Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Duplicate Entries in plotIndexes via `FieldFacet.sow`

Support

FAQs

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

Give us feedback!