Beanstalk: The Finale

Summary

The vulnerability lies in the incorrect declaration of the bdvRemoved variable within the _withdrawDeposit function of the TokenSilo contract. This variable is intended to represent the difference in the base deposit value (BDV) before and after a withdrawal operation. Due to being declared as uint256, it cannot handle negative values, which is a logical oversight given the context of withdrawals potentially decreasing the BDV.

Vulnerability Details

Within the _withdrawDeposit function, the bdvRemoved variable is declared as uint256, which restricts it to storing only non-negative integers. However, in the context of a withdrawal operation, bdvRemoved is supposed to capture the reduction in the BDV, which could be a negative value if the withdrawal exceeds the initial BDV. This discrepancy between the variable's data type and its intended use case leads to a logical error that could affect the accuracy of withdrawal calculations.
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/4e0ad0b964f74a1b4880114f4dd5b339bc69cd3e/protocol/contracts/beanstalk/silo/SiloFacet/TokenSilo.sol#L195-L196

Impact

The primary impact of this vulnerability is on the integrity and accuracy of withdrawal operations within the TokenSilo contract. If bdvRemoved cannot accurately represent decreases in the BDV due to its uint256 declaration, it could lead to incorrect calculations of the remaining balance after a withdrawal.

Tools Used

Manual Code Review

Recommended Mitigation

The bdvRemoved variable should be declared as int256 instead of uint256. This change will allow bdvRemoved to accurately represent both increases and decreases in the BDV, ensuring the correctness of withdrawal calculations