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

Duplicate Entries in plotIndexes via `MarketplaceFacet.fillPodListing`

ControlZ

Bean bug report

Duplicate Entries in plotIndexes via MarketplaceFacet.fillPodListing

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 fillPodListing function in the MarketplaceFacet, it is possible for multiple entries to have the same index.

Root Cause

During the fillPodListing function call, the user filling the pod listing receives the newly purchased pods from the lister based on the following logic:

Let's assume Alice currently owns a plot starting at index 10, ranging from 10-110 (100 pods), and wishes to sell pods 50-110.
When Bob buys pods 50-60, the transfer process is as follows:

  1. Delete the original plot from Alice.

  2. Create a new plot for Alice from the starting index (10) to the start of the purchased pods (50).

  3. Create a new plot for Bob from the start of the purchased pods (50) to the end (60).

  4. Create a new plot for Alice from the end of the purchased pods (60) to the end (110).

However, if Bob buys zero pods while Alice holds the listing from 10-110, the process will look like this:

  1. Delete the original plot from Alice.

  2. Create a new plot for Alice from the starting index (10) to the end (110) (no change for Alice).

  3. Create a new plot for Bob from the starting index (10) to the starting index (10) (the plot size will be zero, but the index will still be pushed to plotIndexes).

There is currently no check in the _fillListing function to prevent buying zero pods.

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 _fillPodListing function to ensure that beanPayAmount > 0:

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

Proof of Concept

To reproduce the issue, add the following test to protocol/test/Marketplace.test.js in the describe("Fill") section and execute it using yarn test --grep "Test Bug":

describe("Test Bug", async function () {
beforeEach(async function () {
this.podListing = PodListing(user.address, 0, 0, 0, 1000, 500000, 0, 0, EXTERNAL);
await mockBeanstalk.connect(user).createPodListing(this.podListing);
this.amountBeansBuyingWith = 250;
​
this.userBeanBalance = await bean.balanceOf(user.address);
this.user2BeanBalance = await bean.balanceOf(user2.address);
​
​
// this part can be repeated as many times as wanted
this.result = await mockBeanstalk
.connect(user2)
.fillPodListing(this.podListing, 0, EXTERNAL);
​
this.result = await mockBeanstalk
.connect(user2)
.fillPodListing(this.podListing, this.amountBeansBuyingWith, EXTERNAL);
​
this.user2BeanBalanceAfter = await bean.balanceOf(user2.address);
this.userBeanBalanceAfter = await bean.balanceOf(user.address);
});
​
it("user2 has 2 plots with the same index", async function () {
const a = await beanstalk.getPlotsFromAccount(user2.address, 0)
console.log(a)
})
});
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Appeal created

ControlZ Submitter
about 1 year ago
inallhonesty Lead Judge
12 months ago
inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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