Beanstalk Part 2
Beanstalk
DeFiHardhat
35,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

The parameters `minFertilizerOut` and `minLPTokensOut` in the `FertilizerFacet::mintFertilizer` function can be `0`

bube

Summary

In FertilizerFacet::mintFertilizer function the input parameters minFertilizerOut and minLPTokensOut can be set to 0.

Vulnerability Details

The input parameters minFertilizerOut and minLPTokensOut in the FertilizerFacet::mintFertilizer are intended to protect against slippage issues.

​
function mintFertilizer(
uint256 tokenAmountIn,
uint256 minFertilizerOut,
uint256 minLPTokensOut
) external payable returns (uint256 fertilizerAmountOut) {
fertilizerAmountOut = _getMintFertilizerOut(tokenAmountIn, LibBarnRaise.getBarnRaiseToken());
​
@> require(fertilizerAmountOut >= minFertilizerOut, "Fertilizer: Not enough bought.");
require(fertilizerAmountOut > 0, "Fertilizer: None bought.");
​
uint128 remaining = uint128(LibFertilizer.remainingRecapitalization().div(1e6)); // remaining <= 77_000_000 so downcasting is safe.
require(fertilizerAmountOut <= remaining, "Fertilizer: Not enough remaining.");
//@audit-note here the id can be duplicated, but this is a known issue
uint128 id = LibFertilizer.addFertilizer(
uint128(s.season.current),
tokenAmountIn,
fertilizerAmountOut,
@> minLPTokensOut
);
C.fertilizer().beanstalkMint(msg.sender, uint256(id), (fertilizerAmountOut).toUint128(), s.bpf);
}
​

The purpose of a minFertilizerOut parameter is to ensure that the user receives a minimum amount of Fertilizer for their input tokens, protecting against unfavorable price movements between the initiation and execution of the transaction:

The minimum amount of Fertilizer to purchase. Protects against a significant Barn Raise Token/USD price decrease.

But if the user sets this to 0, it means that the user can accept any amount of Fertilizer, even if the market conditions change drastically and the actual amount received is far less than expected at the time of transaction submission.

Also, the minLPTokensOut according to the comment is:

The minimum amount of LP tokens to receive after adding liquidity with Barn Raise tokens.

But this parameter is not checked if it is not 0. The function is external and the caller can set the minLPTokensOut to 0.
The FertilizerFacet::mintFertilizer function calls the LibFertilizer::addFertilizer function and pass the minLPTokensOut as fourth parameter:

​
function addFertilizer(
uint128 season,
uint256 tokenAmountIn,
uint256 fertilizerAmount,
uint256 minLP
) internal returns (uint128 id) {
AppStorage storage s = LibAppStorage.diamondStorage();
​
uint128 fertilizerAmount128 = fertilizerAmount.toUint128();
​
// Calculate Beans Per Fertilizer and add to total owed
uint128 bpf = getBpf(season);
s.unfertilizedIndex = s.unfertilizedIndex.add(
fertilizerAmount.mul(bpf)
);
// Get id
id = s.bpf.add(bpf);
// Update Total and Season supply
s.fertilizer[id] = s.fertilizer[id].add(fertilizerAmount128);
s.activeFertilizer = s.activeFertilizer.add(fertilizerAmount);
// Add underlying to Unripe Beans and Unripe LP
@> addUnderlying(tokenAmountIn, fertilizerAmount.mul(DECIMALS), minLP);
// If not first time adding Fertilizer with this id, return
if (s.fertilizer[id] > fertilizerAmount128) return id;
// If first time, log end Beans Per Fertilizer and add to Season queue.
push(id);
emit SetFertilizer(id, bpf);
}
​

Then the LibFertilizer::addFertilizer calls the LibFertilizer::addUnderlying function with third parameter minLP (minLP is minLPTokensOut):

function addUnderlying(uint256 tokenAmountIn, uint256 usdAmount, uint256 minAmountOut) internal {
...
uint256 newLP = IWell(barnRaiseWell).addLiquidity(
tokenAmountsIn,
@> minAmountOut,
address(this),
type(uint256).max
);
...
}

And addUnderlying calls IWell::addLiquidity function and pass the minAmountOut as second parameter. The minAmountOut is the minLP parameter. There is no check if the minAmountOut is not 0.

Impact

The minLPTokensOut is part of the operation that involves adding liquidity, then not checking its value it could indeed mean that there is no slippage protection. Users setting it to zero would effectively disable this protection.

If there is no check to ensure that the number of LP tokens received is greater than or equal to minLPTokensOut, and the user sets minLPTokensOut to zero, the following could happen:

  • The user initiates a mintFertilizer transaction with minLPTokensOut set to zero.

  • The FertilizerFacet contract calculates fertilizerAmountOut and proceeds with the transaction without checking if the LP tokens received from adding liquidity are above minLPTokensOut.

  • In the meantime, there is a significant price movement in the underlying assets of the liquidity pool.

  • The user ends up receiving fewer LP tokens than they would have if minLPTokensOut had been enforced, effectively experiencing slippage.

To mitigate this, the contract should include a check similar to the one for minFertilizerOut, ensuring that the number of LP tokens received is not less than minLPTokensOut. If the actual number of LP tokens is less, the transaction should revert to protect the user from slippage.

Also, if minFertilizerOut is set to 0 by the user, it effectively means there is no minimum threshold for the amount of Fertilizer that must be received. This would disable the slippage protection that minFertilizerOut is supposed to provide.
Let's suppose that a user wants to mint Fertilizer tokens by calling the mintFertilizer function. The user expects to receive a certain amount of Fertilizer. However, the user set minFertilizerOut to 0, either by mistake or because the caller is unaware of the risks:

  • The user submits the transaction with minFertilizerOut set to 0.

  • Before the transaction is mined, the price of the Barn Raise token used to mint Fertilizer drops significantly.

  • The contract calculates fertilizerAmountOut based on the new, lower price.

  • Since minFertilizerOut is 0, the require check in the contract passes regardless of the new amount:
    require(fertilizerAmountOut >= minFertilizerOut, "Fertilizer: Not enough bought.");

  • The transaction succeeds, but the user receives much less Fertilizer than they expected at the time of transaction submission.
    This scenario shows that setting minFertilizerOut to 0 removes the protection against price slippage.

Tools Used

Manual Review

Recommendations

Ensure that the input parameters minFertilizerOut and minLPTokensOut are not set to 0. Also, ensure that the number of received LP tokens is not less than minLPTokensOut.

Updates

Lead Judging Commences

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

Informational/Invalid

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

Informational/Invalid

Support

FAQs

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