DeFiHardhat
35,000 USDC
View results
Submission Details
Severity: low
Invalid

There are no deadline protection in `addUnderlying()` when adding liquidity

Proof of Concept

Take a look at https://github.com/Cyfrin/2024-04-beanstalk-2/blob/27ff8c87c9164c1fbff054be5f22e56f86cdf127/protocol/contracts/libraries/LibFertilizer.sol#L87-L151

function addUnderlying(uint256 tokenAmountIn, uint256 usdAmount, uint256 minAmountOut) internal {
AppStorage storage s = LibAppStorage.diamondStorage();
// Calculate how many new Deposited Beans will be minted
uint256 percentToFill = usdAmount.mul(C.precision()).div(
remainingRecapitalization()
);
uint256 newDepositedBeans;
if (C.unripeBean().totalSupply() > s.u[C.UNRIPE_BEAN].balanceOfUnderlying) {
newDepositedBeans = (C.unripeBean().totalSupply()).sub(
s.u[C.UNRIPE_BEAN].balanceOfUnderlying
);
newDepositedBeans = newDepositedBeans.mul(percentToFill).div(
C.precision()
);
}
// Calculate how many Beans to add as LP
uint256 newDepositedLPBeans = usdAmount.mul(C.exploitAddLPRatio()).div(
DECIMALS
);
// Mint the Deposited Beans to Beanstalk.
C.bean().mint(
address(this),
newDepositedBeans
);
// Mint the LP Beans and add liquidity to the well.
address barnRaiseWell = LibBarnRaise.getBarnRaiseWell();
address barnRaiseToken = LibBarnRaise.getBarnRaiseToken();
C.bean().mint(
address(this),
newDepositedLPBeans
);
IERC20(barnRaiseToken).transferFrom(
msg.sender,
address(this),
uint256(tokenAmountIn)
);
IERC20(barnRaiseToken).approve(barnRaiseWell, uint256(tokenAmountIn));
C.bean().approve(barnRaiseWell, newDepositedLPBeans);
uint256[] memory tokenAmountsIn = new uint256[](2);
IERC20[] memory tokens = IWell(barnRaiseWell).tokens();
(tokenAmountsIn[0], tokenAmountsIn[1]) = tokens[0] == C.bean() ?
(newDepositedLPBeans, tokenAmountIn) :
(tokenAmountIn, newDepositedLPBeans);
uint256 newLP = IWell(barnRaiseWell).addLiquidity(
tokenAmountsIn,
minAmountOut,
address(this),
//@audit
type(uint256).max
);
// Increment underlying balances of Unripe Tokens
LibUnripe.incrementUnderlying(C.UNRIPE_BEAN, newDepositedBeans);
LibUnripe.incrementUnderlying(C.UNRIPE_LP, newLP);
s.recapitalized = s.recapitalized.add(usdAmount);
}

This function is used if there is a logic that includes token contributions and so on, case however is that when attempting to add liquidity the execution does not provide all necessary protection, in this instance the protocol uses a hardcoded type(uint256).max as the deadline parameter for this attempt at adding the liquidity via the well, which allows the trnasaction to linger in the mempool for long and cause it to get finalized in a worse condition.

A simple POC of the issue with not having deadline checks, is when, say an attempt is made to swap ABC for XYZ, where the min acceptable XYZ is 100, now if deadline is not provided, this transaction can linger for long where when the trasnaction eveuntually gets fialized, auser could have encountered loss in USD value of XYZ.

Now pertaining to the attached snippet this would also be applicable to the value of the barnRaiseToken that is sent, i.e tokenAmountIn, cause dependent on the current USD value of the token, different decisions would be made on how much tokens to get sent and used to add liquidity.

Impact

Adding liquidity is done without deadline protection could cause for the transaction to be unfairly executed.

Recommended Mitigation Steps

As a general rule, whenever adding/removing liquidity, always attach both slippage & deadline protection.

Updates

Lead Judging Commences

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

Fertilizer deadline

Support

FAQs

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