Beanstalk: The Finale

Summary

The identified issue is primarily due to the functions ConvertFacet.convert, LibPipelineConvert.checkForValidConvertAndUpdateConvertCapacity, LibPipelineConvert.getConvertState, and LibSilo._mow. These functions are responsible for handling the conversion process, validating and updating conversion capacities, determining the conversion state, and updating stalk balances, respectively. They currently do not re-evaluate deltaB and penalties after each conversion within a single transaction. This oversight allows users to bypass penalties by performing multiple consecutive conversions, leading to inaccurate penalty applications and potential financial exploitation of the system.

https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/8c8710df547f7d7c5dd82c5381eb6b34532e4484/protocol/contracts/libraries/Silo/LibSilo.sol#L458C3-L482C6

https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/8c8710df547f7d7c5dd82c5381eb6b34532e4484/protocol/contracts/beanstalk/silo/ConvertFacet.sol#L55C1-L105C2

https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/8c8710df547f7d7c5dd82c5381eb6b34532e4484/protocol/contracts/libraries/Convert/LibPipelineConvert.sol#L186C4-L213C10

https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/8c8710df547f7d7c5dd82c5381eb6b34532e4484/protocol/contracts/libraries/Convert/LibPipelineConvert.sol#L158C1-L180C6

Proof of Concept

1: Initial Setup:

• The user holds a balance of tokens eligible for conversion.

• The system is configured with certain penalties and conversion capacities.

2: Performing Multiple Conversions:

• The user initiates a transaction that calls ConvertFacet.convert to convert a portion of their tokens.

• The convert function invokes LibPipelineConvert.getConvertState to determine the initial conversion state, including deltaB and conversion capacity.

• The conversion is processed, and the LibPipelineConvert.checkForValidConvertAndUpdateConvertCapacity function is called to update the conversion capacity and check for any penalties.

• However, since the convert function does not re-evaluate deltaB and penalties after each conversion within the transaction, the penalties applied are based on outdated data.

3: Consecutive Conversions:

• Within the same transaction, the user performs another conversion using ConvertFacet.convert.

• Again, LibPipelineConvert.getConvertState and LibPipelineConvert.checkForValidConvertAndUpdateConvertCapacity are called, but they do not account for the changes from the first conversion.

• LibSilo._mow is called to update stalk balances, but it also does not handle consecutive calls within a single transaction correctly.

• The user can repeat this process multiple times within the same transaction.

4: Outcome:

• Due to the lack of re-evaluation of deltaB and penalties after each conversion, the user successfully bypasses the intended penalties.

• The user gains an unfair advantage by performing conversions without incurring the correct penalties, leading to potential financial exploitation of the system.

Impact

Users can exploit this vulnerability to avoid penalties, leading to potential financial losses for the protocol and unfair advantages for certain users.

Tools Used

Manual Review

Recommendations

1: Ensure that each conversion re-evaluates the deltaB accurately after each conversion within a single transaction.

2: Introduce logic to accurately track and update the deltaB after each conversion step.

3: Implement a mechanism to prevent cumulative conversions that avoid penalties by splitting them across multiple transactions.