Summary
The FertilizerFacet:mintFertilizer calculate the Fertilizer token to mint and also calls the Will.addLiquidity() function to add new tokens to liquidity, However there is no deadline check in place to prevent transaction execution when it is not in favor of user and it also passes the type(uint256).max as deadline to addLiquidity function external call.
Vulnerability Details
The MintFertilizer function has two Issues:
-
Whenever user calls mintFertilizer to mint New fertilizer token. The Protocol takes the minLPTokensOut as arguments and pass it to addFertilizer function. The addFertilizer function pass type(uint256).max as a deadline for transaction execution at Pool contract.
-
There is no deadline checks on mintFertilizer transaction execution as the code tries to calculate the amount of fertilizer token to be minted for user using Oracle Price.
Both of these vulnerabilities will lead to miner attack. In the 1st case the miner only need to check for price drop to a certain level where the minFertilizerOut amount could be satisfied. where in second 2nd case the miner need to wait for newLP minted will satisfy minLPTokensOut.
-
Vulnerable Code for 1st case:
function addUnderlying(uint256 tokenAmountIn, uint256 usdAmount, uint256 minAmountOut) internal {
...
uint256 newLP = IWell(barnRaiseWell).addLiquidity(
tokenAmountsIn,
minAmountOut,
address(this),
@> type(uint256).max
);
LibUnripe.incrementUnderlying(C.UNRIPE_BEAN, newDepositedBeans);
LibUnripe.incrementUnderlying(C.UNRIPE_LP, newLP);
s.recapitalized = s.recapitalized.add(usdAmount);
}
Vulnerable Code for 2nd case:
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));
require(fertilizerAmountOut <= remaining, "Fertilizer: Not enough remaining.");
uint128 id = LibFertilizer.addFertilizer(
uint128(s.season.current),
tokenAmountIn,
fertilizerAmountOut,
minLPTokensOut
);
C.fertilizer().beanstalkMint(msg.sender, uint256(id), (fertilizerAmountOut).toUint128(), s.bpf);
}
POC :
The user submit transaction to mintFertilizer with following arguments:
tokenAmountIn=10
minFertilizerOut=9
minLPTokensOut=9
At the time when user submit transaction the user could receive 9.5 Fertilizer tokens and add 9.5 as a LP token to Pool. as we know that there are no deadline so the miner can hold the transaction for a wait when it is possible to mint exact 9 Fertilizer token and add 9 tokens to Liquidity Pool, then miner execute the transaction.
Impact
The user minted Fertilizer token and New Liquidity will be tricked to forcefully mint min amount.
Tools Used
Manual Review
Recommendations
The Possible fix for the vulnerability could be as follows:
Add Following changes to FertilizerFacet.sol file:
// add this modifier
+ modifier ensureDeadline(uint32 deadline) {
+ require(block.timestamp < deadline, "Deadline has passed");
+ _;
+ }
...
@@ -64,8 +69,9 @@ contract FertilizerFacet {
function mintFertilizer(
uint256 tokenAmountIn,
uint256 minFertilizerOut,
- uint256 minLPTokensOut
- ) external payable returns (uint256 fertilizerAmountOut) {
+ uint256 minLPTokensOut,
+ uint32 deadline
+ ) external ensureDeadline(deadline) payable returns (uint256 fertilizerAmountOut) {
fertilizerAmountOut = _getMintFertilizerOut(tokenAmountIn, LibBarnRaise.getBarnRaiseToken());
require(fertilizerAmountOut >= minFertilizerOut, "Fertilizer: Not enough bought.");
@@ -78,7 +84,8 @@ contract FertilizerFacet {
uint128(s.season.current),
tokenAmountIn,
fertilizerAmountOut,
- minLPTokensOut
+ minLPTokensOut,
+ deadline
);
C.fertilizer().beanstalkMint(msg.sender, uint256(id), (fertilizerAmountOut).toUint128(), s.bpf);
Add Following changes to LibFertilizer.sol file:
@@ -44,7 +45,8 @@ library LibFertilizer {
uint128 season,
uint256 tokenAmountIn,
uint256 fertilizerAmount,
- uint256 minLP
+ uint256 minLP,
+ uint32 deadline
) internal returns (uint128 id) {
AppStorage storage s = LibAppStorage.diamondStorage();
@@ -61,7 +63,7 @@ library LibFertilizer {
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);
+ addUnderlying(tokenAmountIn, fertilizerAmount.mul(DECIMALS), minLP, deadline);
// 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.
@@ -84,7 +86,7 @@ library LibFertilizer {
* @dev Any token contributions should already be transferred to the Barn Raise Well to allow for a gas efficient liquidity
* addition through the use of `sync`. See {FertilizerFacet.mintFertilizer} for an example.
*/
- function addUnderlying(uint256 tokenAmountIn, uint256 usdAmount, uint256 minAmountOut) internal {
+ function addUnderlying(uint256 tokenAmountIn, uint256 usdAmount, uint256 minAmountOut,uint32 deadline) internal {
...
@@ -136,16 +141,17 @@ library LibFertilizer {
(newDepositedLPBeans, tokenAmountIn) :
(tokenAmountIn, newDepositedLPBeans);
- uint256 newLP = IWell(barnRaiseWell).addLiquidity(
+ uint256 newLP = IWell(barnRaiseWell).addLiquidity( // call addLiquidityFeeOnTransfer
tokenAmountsIn,
minAmountOut,
address(this),
- type(uint256).max
+ deadline
);
