Beanstalk Part 3

Summary

If twaDeltaB is negative but instDeltaB is positive, then during the execution of Sun.setSoilBelowPeg(), soil will be overissued due to overflow and wrap around of instDeltaB in the computation of uint256(-instDeltaB).

Vulnerability Details

At the start of a new season, soil is issued when the bean value is below the peg. The amount of soil issued depends on how far below the peg the beans are. As explained in the README, the previous version of the code used only the time-weighted average deltaB (twaDeltaB), which sometimes resulted in the overissuance of soil. To avoid this, the new implementation compares the time-weighted average deltaB (twaDeltaB) with the instantaneous deltaB (instDeltaB) and uses the minimum of the two to compute how much soil to issue. (See code below)

However, this approach assumes that if twaDeltaB is negative, then instDeltaB is also negative, which is not always the case. If twaDeltaB is negative but instDeltaB is positive, then in the expression Math.min(uint256(-twaDeltaB), uint256(-instDeltaB)), -instDeltaB will be a negative value. Consequently, uint(-instDeltaB) will overflow and wrap around, resulting in very large values. As a result, uint256(-twaDeltaB) will be incorrectly chosen as the minimum when it shouldn't be, leading to the issuance of soil when it shouldn't occur.

Impact

Soil will be overissued if twaDeltaB is negative but instDeltaB is positive.

Tools Used

Manual Review.

Recommendations

Consider checking if instDeltaB is negative before applying the negation operator (-), thus preventing the wrap around.