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

`sopWell` conducts swaps in well without slippage or deadline protections

zanybonzy

Relevant GitHub Links

https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/df2dd129a878d16d4adc75049179ac0029d9a96b/protocol/contracts/libraries/Silo/LibFlood.sol#L299

Summary

Swaps are conducted in the well when handling rain, with no slippage protection and an extremely large deadline parameter which can lead to sandwich attacks and loss of slippage.

Vulnerability Details

When the handleRain function is called, it attempts to handled the "season of plenty" by calling the sopWell function. The function helps to handle oversaturation and helps bring bean price closer to peg by minting beans and swapping them in the well through the swapFrom function. The issue is that this swap is conducted with basically no protection from slippage and a large deadline parameter.

From LibFlood.sol

function sopWell(WellDeltaB memory wellDeltaB) private {
AppStorage storage s = LibAppStorage.diamondStorage();
if (wellDeltaB.deltaB > 0) {
IERC20 sopToken = LibWell.getNonBeanTokenFromWell(wellDeltaB.well);
​
uint256 sopBeans = uint256(wellDeltaB.deltaB);
C.bean().mint(address(this), sopBeans);
​
// Approve and Swap Beans for the non-bean token of the SOP well.
C.bean().approve(wellDeltaB.well, sopBeans);
uint256 amountOut = IWell(wellDeltaB.well).swapFrom(
C.bean(),
sopToken,
sopBeans,
0,
address(this),
type(uint256).max
);
rewardSop(wellDeltaB.well, amountOut, address(sopToken));
emit SeasonOfPlentyWell(
s.sys.season.current,
wellDeltaB.well,
address(sopToken),
amountOut
);
}
}

and from IWell.sol

function swapFrom(
IERC20 fromToken,
IERC20 toToken,
uint256 amountIn,
uint256 minAmountOut,
address recipient,
uint256 deadline
) external returns (uint256 amountOut);

As can be seen from the functions, the library uses a minAmountOut parameter of 0, meaning any amount including 0 returned is acceptable. It also uses a deadline parameter of type(uint256).max which means that the transaction can stay for that long in the mempool, held down by malicious miners.
Since the protocol aims to deploy on ethereum mainnet, which has a public mempool and is notorious for lots of MEV bots, malicious users and these bots can simply monitor and sandwich the handleRain transactions for a quick profit leaving the protcol at a loss.

Impact

Loss of funds due to sandwich attacks.

Tools Used

Manual Code Review

Recommendations

Recommend using more reasonable parameters instead of 0. A certain percentage of the amount being swapped can be used instead.

Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

Lack of slippage in sopWell

Support

FAQs

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