Beanstalk Part 3

Summary

When doing calculations for chop rate, there should be no scenario where the math breaks. However, when the function uses the totalSupply() value, it is not checked
if it is greater than zero. This can lead to unexpected error's in the contract accounting calculation of chop rate.

1. The totalSupply is passed to LibFertilizer.getTotalRecapDollarsNeeded(supply) if the unripeToken is C.UNRIPE_LP.

2. This determines the total recapitalization dollars needed based on the current supply of the unripe LP tokens.

3. The core calculation involves determining how much of the underlying token can be redeemed based on several factors, including supply.

Using the following formula: redeem = (underlyningAmount x s.recapitilized/totalUSDneeded x amount/supply)

i.e. redeem = underlyingAmount.mul(s.recapitalized).div(totalUsdNeeded).mul(amount).div(supply); line in getPenalizedUnderlyning.

Vulnerability Details

The function chop in UnripeFacet.sol:
https://github.com/Cyfrin/2024-05-Beanstalk-3/blob/main/protocol/contracts/beanstalk/barn/UnripeFacet.sol#L79C1-L100C6

The following code:

calls function LibChop.chop

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

which pulls the underlyingAmount amount with the line underlyingAmount = LibUnripe.getPenalizedUnderlying(unripeToken, amount, supply);

Found in the Library, LibUnripe.sol:

https://github.com/Cyfrin/2024-05-Beanstalk-3/blob/main/protocol/contracts/libraries/LibUnripe.sol#L149

Which finally pulls the

https://github.com/Cyfrin/2024-05-Beanstalk-3/blob/main/protocol/contracts/libraries/LibFertilizer.sol#L232

getTotalRecapDollarsNeeded() which is stored as variable totalSupply();

Impact

I reported this as medium, because contract accounting is critical. totalSupply() set to zero can easily happen at the initial states of the contract and can cause contract to crash. When doing calculations for chop rate, all edge cases that can cause accounting to break should be handled.

1. Division by Zero: The calculation in getPenalizedUnderlying relies on dividing by totalUsdNeeded. If totalSupply is zero, totalUsdNeeded will also be zero, leading to a division by zero error when the function is called with unripeToken as an unripe LP token. This can crash the contract or lead to undefined behavior.

2. Inaccurate Calculations: Even if the function doesn't crash, a zero total supply would result in inaccurate calculations for the penalty and exchange rate. This could disadvantage users or lead to unintended consequences.

3. Unexpected Behavior: The contract might behave differently than intended with a zero total supply. This could make it harder to debug and audit the contract's functionality.

Tools Used

Manual Analysis, Sol2Uml and VS-Code Surya

Recommendations

Add a check that ensures that totalSupply() is greater than 0: