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

Malicious users can delete plots from other users in a specific edge case

deadrosesxyz

Summary

Malicious users can delete plots from other users in a specific edge case

Vulnerability Details

Let's look at the code of _transferPlot

function _transferPlot(
address from,
address to,
uint256 fieldId,
uint256 index,
uint256 start,
uint256 amount
) internal {
require(from != to, "Field: Cannot transfer Pods to oneself.");
insertPlot(to, fieldId, index + start, amount);
removePlot(from, fieldId, index, start, amount + start);
emit PlotTransfer(from, to, index + start, amount);
}
​
function insertPlot(address account, uint256 fieldId, uint256 index, uint256 amount) internal {
s.accts[account].fields[fieldId].plots[index] = amount;
s.accts[account].fields[fieldId].plotIndexes.push(index);
}

As we can see, the way transfer works, is that the receiver's index + start slot is overwritten to its new value. This could be problematic, if a user manages to invoke a transfer for an index which the receiver already has.

Though, since two users cannot possibly have plots on the same index, the only way this could be executed is through a 0-value transfer.

function _fillPodOrder(
PodOrder calldata o,
uint256 index,
uint256 start,
uint256 amount,
LibTransfer.To mode
) internal {
​
require(amount >= o.minFillAmount, "Marketplace: Fill must be >= minimum amount.");
require(s.a[msg.sender].field.plots[index] >= (start.add(amount)), "Marketplace: Invalid Plot.");
require(index.add(start).add(amount).sub(s.f.harvestable) <= o.maxPlaceInLine, "Marketplace: Plot too far in line.");
bytes32 id = createOrderId(o.account, o.pricePerPod, o.maxPlaceInLine, o.minFillAmount);
uint256 costInBeans = amount.mul(o.pricePerPod).div(1000000);
s.podOrders[id] = s.podOrders[id].sub(costInBeans, "Marketplace: Not enough beans in order.");
​
LibTransfer.sendToken(C.bean(), costInBeans, msg.sender, mode);
if (s.podListings[index] != bytes32(0)) _cancelPodListing(msg.sender, index);
_transferPlot(msg.sender, o.account, index, start, amount);
​
if (s.podOrders[id] == 0) delete s.podOrders[id];
emit PodOrderFilled(msg.sender, o.account, id, index, start, amount, costInBeans);
}

As we can see, the only restriction _fillPodOrder has is that the filled amount is at least the minFillAmount. Meaning that if minFillAmount == 0, a 0-value fill amount will succeed.

Here's how an attacker can utilize the system:

  1. Attacker get some plots

  2. Attacker creates a podOrder from another wallet with minFillAmount == 0.

  3. Attacker fills the podOrder with 0. This will add the needed index to the new wallet's plotIndexes

  4. Attacker creates a listing for their plots

  5. Victim fills the listing and buys attacker's plots.

  6. Now if victim has/creates a podOrder with minFillAmount == 0, the attacker can fill it with 0 and override the just bought plots, deleting them.

Impact

Loss of plots

Tools Used

Manual review

Recommendations

Never allow 0-value plot transfers

Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational/Gas

Invalid as per docs https://docs.codehawks.com/hawks-auditors/how-to-determine-a-finding-validity

Appeal created

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

Malicious users can delete plots from other users in a specific edge case

Support

FAQs

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