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

`amountOut` not compared against `minAmountOut` in both `convertLPToBeans()` and `convertBeansToLP()` functions before proceeding with minting

tigerfrake

Summary

In both convertLPToBeans() and convertBeansToLP() functions, there's a calculation for minAmountOut, which represents the minimum amount of the output token that should be received based on the provided input. This value is calculated based on the minimum acceptable amount of the other token (either beans or LP tokens), ensuring that the conversion meets certain requirements.

Vulnerability Details

convertBeansToLP()

  • https://github.com/Cyfrin/2024-04-beanstalk-2/blob/27ff8c87c9164c1fbff054be5f22e56f86cdf127/protocol/contracts/libraries/Convert/LibUnripeConvert.sol#L59-L94

function convertBeansToLP(bytes memory convertData)
internal
returns (
address tokenOut,
address tokenIn,
uint256 amountOut,
uint256 amountIn
)
{
tokenIn = C.UNRIPE_BEAN;
tokenOut = C.UNRIPE_LP;
(uint256 beans, uint256 minLP) = convertData.basicConvert();
uint256 minAmountOut = LibUnripe
.unripeToUnderlying(tokenOut, minLP, IBean(C.UNRIPE_LP).totalSupply())
.mul(LibUnripe.percentBeansRecapped())
.div(LibUnripe.percentLPRecapped());
(
uint256 outUnderlyingAmount,
uint256 inUnderlyingAmount
) = LibWellConvert._wellAddLiquidityTowardsPeg(
LibUnripe.unripeToUnderlying(tokenIn, beans, IBean(C.UNRIPE_BEAN).totalSupply()),
minAmountOut,
LibBarnRaise.getBarnRaiseWell()
);
​
​
amountIn = LibUnripe.underlyingToUnripe(tokenIn, inUnderlyingAmount);
LibUnripe.removeUnderlying(tokenIn, inUnderlyingAmount);
IBean(tokenIn).burn(amountIn);
​
​
amountOut = LibUnripe
.underlyingToUnripe(tokenOut, outUnderlyingAmount)
.mul(LibUnripe.percentLPRecapped())
.div(LibUnripe.percentBeansRecapped());
LibUnripe.addUnderlying(tokenOut, outUnderlyingAmount);
IBean(tokenOut).mint(address(this), amountOut);
}

convertLPToBeans()

  • https://github.com/Cyfrin/2024-04-beanstalk-2/blob/27ff8c87c9164c1fbff054be5f22e56f86cdf127/protocol/contracts/libraries/Convert/LibUnripeConvert.sol#L22-L57

function convertLPToBeans(bytes memory convertData)
internal
returns (
address tokenOut,
address tokenIn,
uint256 amountOut,
uint256 amountIn
)
{
tokenOut = C.UNRIPE_BEAN;
tokenIn = C.UNRIPE_LP;
(uint256 lp, uint256 minBeans) = convertData.basicConvert();
uint256 minAmountOut = LibUnripe
.unripeToUnderlying(tokenOut, minBeans, IBean(C.UNRIPE_BEAN).totalSupply())
.mul(LibUnripe.percentLPRecapped())
.div(LibUnripe.percentBeansRecapped());
(
uint256 outUnderlyingAmount,
uint256 inUnderlyingAmount
) = LibWellConvert._wellRemoveLiquidityTowardsPeg(
LibUnripe.unripeToUnderlying(tokenIn, lp, IBean(C.UNRIPE_LP).totalSupply()),
minAmountOut,
LibBarnRaise.getBarnRaiseWell()
);
​
​
amountIn = LibUnripe.underlyingToUnripe(tokenIn, inUnderlyingAmount);
LibUnripe.removeUnderlying(tokenIn, inUnderlyingAmount);
IBean(tokenIn).burn(amountIn);
​
​
amountOut = LibUnripe
.underlyingToUnripe(tokenOut, outUnderlyingAmount)
.mul(LibUnripe.percentBeansRecapped())
.div(LibUnripe.percentLPRecapped());
LibUnripe.addUnderlying(tokenOut, outUnderlyingAmount);
IBean(tokenOut).mint(address(this), amountOut);
}

After calculating minAmountOut, the functions proceed to execute the conversion by calling appropriate liquidity-related functions (_wellRemoveLiquidityTowardsPeg() in convertLPToBeans() and _wellAddLiquidityTowardsPeg() in convertBeansToLP()). These functions interact with the liquidity pools to perform the conversion operation.

The critical part here is the subsequent minting operation:

IBean(tokenOut).mint(address(this), amountOut);

This line mints the output token (tokenOut) to the current contract (address(this)) with the calculated amountOut. However, there's no explicit comparison between amountOut and minAmountOut before proceeding with this minting operation.

The absence of a comparison between amountOut and minAmountOut means there's no check to ensure that the actual output amount (amountOut) meets the minimum required output (minAmountOut).

Impact

  • Loss of Funds:
    If the actual output amount is lower than the minimum required amount, the amount minted would be lower than the expected output, potentially resulting in financial losses.

Tools Used

Manual Review

Recommendations

Include a comparison between amountOut and minAmountOut before proceeding with the minting operation. If amountOut is less than minAmountOut, appropriate actions should be taken, such as reverting the transaction.

// Check if the actual output amount meets the minimum required output
require(amountOut>= minAmountOut, "Insufficient output amount");
Updates

Lead Judging Commences

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

Informational/Invalid

Support

FAQs

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