Beanstalk Part 3

Summary

The Beanstalk protocol's current mechanism for converting unripe tokens into their ripe counterparts, lacks any slippage protection whatsoever. This oversight means users may receive significantly less than expected when converting unripe tokens to ripe tokens, with the system accepting any value > = 1 wei as mathematically valid, regardless of the intended transaction amount provided by the users.

This situation arises because the calculation for determining the amount of ripe tokens to return does not consider market conditions or allow for adjustments, albeit it leading to potential losses for users who rely on the accuracy of the conversion rate.

Vulnerability Details

First see https://github.com/Cyfrin/2024-05-Beanstalk-3/blob/662d26f12ee219ee92dc485c06e01a4cb5ee8dfb/protocol/contracts/beanstalk/barn/UnripeFacet.sol#L79-L100

Now see https://github.com/Cyfrin/2024-05-Beanstalk-3/blob/662d26f12ee219ee92dc485c06e01a4cb5ee8dfb/protocol/contracts/libraries/Convert/LibChopConvert.sol#L27-L46

These functions are used to convert an unripe asset to it's ripe counterpart where the latter takes care of converting deposited Unripe tokens into their deposited Ripe Tokens and the former chops the asset according to the recapitalization % after burning the amountIn, note that both functions delegate the heavy conversion to LibChop.chop https://github.com/Cyfrin/2024-05-Beanstalk-3/blob/662d26f12ee219ee92dc485c06e01a4cb5ee8dfb/protocol/contracts/libraries/LibChop.sol#L27-L37

This function does the "chopping" or converting of the Unripe Token into its Ripe Token, the underlying amount being removed is gotten from LibUnripe.getPenalizedUnderlying() and the amount from LibChopConvertconvertUnripeToRipe() is being passed to this function to get the underlyingAmount for the provided amount.

Now here is the implementation of LibUnripe.getPenalizedUnderlying() https://github.com/Cyfrin/2024-05-Beanstalk-3/blob/662d26f12ee219ee92dc485c06e01a4cb5ee8dfb/protocol/contracts/libraries/LibUnripe.sol#L149-L171

Evidently the if(redeem > underlyingAmount) redeem = underlyingAmount; translates to the fact that the reduction has been "hardcoded" and essentially this process lacks any slippage which is necessary for any swap.

That's to say this implementation would cause for the wrong amount of underlying to be removed via LibUnripe, essentially showcasing that for the amount of Unripe Token passed into LibChop#chop() the wrong rates is used to change it into its Ripe Token.

Now since this attempt at converting lacks any slippage whatsoever the attempt at sending users the tokens could return as low as 1 wei and there is no way for a user to decline this https://github.com/Cyfrin/2024-05-Beanstalk-3/blob/662d26f12ee219ee92dc485c06e01a4cb5ee8dfb/protocol/contracts/beanstalk/barn/UnripeFacet.sol#L95-L96

Impact

Users would lose out on their unripe asset when chopping it into its ripe counterpart since no slippage whatsoever is applied, and any value above 1 is acceptable for whatever amount of unripe tokens that gets burnt for the users, which indicates that even a slippage of 99% is unfairly acceptable for users.

This also falls on the "Contract fails to return promised returns" since the documentation clearly state that this calculation is according to the new chop rate which is %Recapitalized^2, albeit this is of a higher severity as it causes user's loss of funds, cause users who do the calculation before hand with the rate in mind would receive lesser tokens and they can't reject the reduction, keep in mind that if they knew this reduction would be done before hand, they can just reduce the amount that's been chopped to ensure their redemption is not heavily reduced, since the amount that's passed to be chopped has a direct relation to the amount that ends up being redeemed: https://github.com/Cyfrin/2024-05-Beanstalk-3/blob/662d26f12ee219ee92dc485c06e01a4cb5ee8dfb/protocol/contracts/libraries/LibUnripe.sol#L167

Tools Used

Manual review

Recommendations

Consider making the attempt at chopping the unripe tokens being exactly changed for the amount of ripe tokens, in this case if (redeem > underlyingAmount) redeem = underlyingAmount; then this should also reflect in the amount of unripe tokens chopped, i.e the amount passed into LibUnripe.getPenalizedUnderlying() should be reduced to cover the available underlying amount and essentially the amount of unripe tokens chopped via LibChop.chop() should also be reduced.

TLDR: Apply a slippage parameter to UnripeFacet#chop() and check to see if the underlying amount returned is >= to this slippaged value provided by the user and if yes, allow their transactions to succeed.