Summary
The FieldFacet public harvest function lacks an explicit ownership verification, allowing malicious users to harvest Pods from plots they do not own. This vulnerability can lead to unauthorized harvesting, economic disruption, and loss of trust in the protocol. The absence of an explicit check to ensure that the caller owns the plots they are attempting to harvest allows malicious users to exploit this vulnerability and harvest Pods from plots they do not own.
Current harvest Function:
function harvest(uint256 fieldId, uint256[] calldata plots, LibTransfer.To mode) external payable fundsSafu noSupplyChange oneOutFlow(C.BEAN) {
uint256 beansHarvested = _harvest(fieldId, plots);
LibTransfer.sendToken(C.bean(), beansHarvested, LibTractor._user(), mode);
}
_harvest Function:
function _harvest(uint256 fieldId, uint256[] calldata plots) internal returns (uint256 beansHarvested) {
for (uint256 i; i < plots.length; ++i) {
require(plots[i] < s.sys.fields[fieldId].harvestable, "Field: Plot not Harvestable");
uint256 harvested = _harvestPlot(LibTractor._user(), fieldId, plots[i]);
beansHarvested += harvested;
}
s.sys.fields[fieldId].harvested += beansHarvested;
emit Harvest(LibTractor._user(), fieldId, plots, beansHarvested);
}
_harvestPlot Function:
function _harvestPlot(
address account,
uint256 fieldId,
uint256 index
) private returns (uint256 harvestablePods) {
uint256 pods = s.accts[account].fields[fieldId].plots[index];
require(pods > 0, "Field: no plot");
harvestablePods = s.sys.fields[fieldId].harvestable.sub(index);
LibMarket._cancelPodListing(LibTractor._user(), fieldId, index);
delete s.accts[account].fields[fieldId].plots[index];
LibDibbler.removePlotIndexFromAccount(account, fieldId, index);
if (harvestablePods >= pods) {
return pods;
}
uint256 newIndex = index.add(harvestablePods);
s.accts[account].fields[fieldId].plots[newIndex] = pods.sub(harvestablePods);
s.accts[account].fields[fieldId].plotIndexes.push(newIndex);
}
Proof of Concept
Consider a scenario where a malicious user exploits the lack of ownership verification in the harvest function to harvest Pods from a plot they do not own.
The malicious user calls the harvest function with the victim's plot details, resulting in unauthorized harvesting of the Pods.
Test
pragma solidity ^0.8.20;
import "forge-std/Test.sol";
import "../contracts/Beanstalk.sol";
contract BeanstalkTest is Test {
Beanstalk beanstalk;
address victim = address(0x1);
address maliciousUser = address(0x2);
uint256 fieldId = 1;
uint256 plotIndex = 123;
function setUp() public {
beanstalk = new Beanstalk();
beanstalk.addPlot(victim, fieldId, plotIndex, 100);
}
function testUnauthorizedHarvest() public {
vm.prank(maliciousUser);
vm.expectRevert("Field: caller does not own the plot");
beanstalk.harvest(fieldId, [plotIndex], LibTransfer.To.EXTERNAL);
}
function testAuthorizedHarvest() public {
vm.prank(victim);
beanstalk.harvest(fieldId, [plotIndex], LibTransfer.To.EXTERNAL);
}
}
Impact
This oversight can lead to potential unauthorized access, allowing users to harvest plots they do not own. This compromises the integrity and security of the system, as it enables malicious actors to exploit this vulnerability to gain unauthorized benefits.
Tools Used
Manual Review
Recommendations
1: Ensure the harvest function verifies that the caller owns the plots they are attempting to harvest.
Updated harvest Function with Ownership Check:
function harvest(uint256 fieldId, uint256[] calldata plots, LibTransfer.To mode) external payable fundsSafu noSupplyChange oneOutFlow(C.BEAN) {
for (uint256 i; i < plots.length; ++i) {
require(s.accts[msg.sender].fields[fieldId].plots[plots[i]] > 0, "Field: caller does not own the plot");
}
uint256 beansHarvested = _harvest(fieldId, plots);
LibTransfer.sendToken(C.bean(), beansHarvested, LibTractor._user(), mode);
}
