Beanstalk Part 3

Summary

LibChop.chop() makes a cross-contract call to LibUnripe.removeUnderlying(), which incorrectly decreases the s.recapitalized state variable when chopping Unripe LP tokens. This leads to an inaccurate tracking of the total recapitalized amount.

Vulnerability Details

The issue lies in the LibUnripe.removeUnderlying() function, which is called by LibChop.chop() during the chopping of Unripe tokens:

https://github.com/Cyfrin/2024-05-Beanstalk-3/blob/662d26f12ee219ee92dc485c06e01a4cb5ee8dfb/protocol/contracts/libraries/LibUnripe.sol#L123-L132

When chopping Unripe LP tokens, the s.recapitalized variable is decremented proportionally to the amount of underlying tokens being removed. However, s.recapitalized represents the total dollar value of Fertilizer used for recapitalization, and it should not be affected by the chopping of Unripe tokens.

The LibChop.chop() function calls removeUnderlying() without considering the impact on s.recapitalized:

https://github.com/Cyfrin/2024-05-Beanstalk-3/blob/662d26f12ee219ee92dc485c06e01a4cb5ee8dfb/protocol/contracts/libraries/LibChop.sol#L27-L38

The comment on line 34 in LibChop.sol

indicates that the intention is to remove the underlying amount and decrease s.recapitalized if the token is an unripe LP token. Albeit, this behavior is not aligned with the expected logic for a Chop operation, since s.recapitalized should not be decreased during a Chop. This comment and the corresponding code in LibUnripe.removeUnderlying() should be reviewed to make sure that s.recapitalized remains accurate and reflects the total dollar value of Fertilizer used for recapitalization, independent of Chops.

Impact

Decrementing s.recapitalized during a Chop of Unripe LP tokens incorrectly reduces the tracked recapitalized amount, even though no recapitalization is happening. This causes an inaccurate representation of the total recapitalized value, which affects other parts of the protocol that rely on this value.

Tools Used

Manual review

Recommended Mitigation Steps

Consider modifyiing the LibUnripe.removeUnderlying function to skip the s.recapitalized update logic when the function is being called as part of a Chop. This can be done by adding a boolean parameter to indicate whether the call is coming from a Chop or not.
Alternatively, adjust the Unripe balance management logic to track the Unripe LP underlying value separately from s.recapitalized. This way, chopping Unripe LP tokens won't affect the recapitalized amount.