Report #34649

Report Date

August 19, 2024

Status

Confirmed

Payout

3,000

The rounding up of the BDV removal during silo withdraws and transfers does not actually round up

‣

Report Info

Report ID

#34649

Report type

Smart Contract

Has PoC?

Yes

Target

https://etherscan.io/address/0xC1E088fC1323b20BCBee9bd1B9fC9546db5624C5

Impacts

Illegitimate minting of protocol native assets

Description

When a silo withdraw or transfer is executed, the amount of the deposited token is discounted from the user's position and the proportional BDV needs to be removed also from the user because it is used in order to compute the amount of stalk to account to the user when he mows. This BDV computation is intended to be rounded up against the user in order to always have more value deposited than bdv accounted. However, this computation does not round up and can be exploited by a user to have a greater bdv accounted in his position than the value of the deposited amount.

Vulnerability Details

In particular, the function to compute the bdv to remove is the following one:

    function removeDepositFromAccount(
        address account,
        address token,
        int96 stem,
        uint256 amount
    ) internal returns (uint256 crateBDV) {
        AppStorage storage s = LibAppStorage.diamondStorage();
        uint256 depositId = LibBytes.packAddressAndStem(token, stem);

        uint256 crateAmount = s.a[account].deposits[depositId].amount;
        crateBDV = s.a[account].deposits[depositId].bdv;

        ...

        // Partial remove
        if (amount < crateAmount) {
            // round up removal of BDV. (x - 1)/y + 1
            // https://stackoverflow.com/questions/17944
            uint256 removedBDV = amount.sub(1).mul(crateBDV).div(crateAmount).add(1);
            uint256 updatedBDV = crateBDV.sub(removedBDV);
            uint256 updatedAmount = crateAmount.sub(amount);

            // SafeCast unnecessary b/c updatedAmount <= crateAmount and updatedBDV <= crateBDV,
            // which are both <= type(uint128).max
            s.a[account].deposits[depositId].amount = updatedAmount.toUint128();
            s.a[account].deposits[depositId].bdv = updatedBDV.toUint128();

            s.a[account].mowStatuses[token].bdv = s.a[account].mowStatuses[token].bdv.sub(
                removedBDV.toUint128()
            );

            return removedBDV;
        }
        ...
    }

As we can see in this computation, the math used is: (amountToRemove - 1)*BDV totalAmountDeposited + 1. However, this computation returns values that will be less than expected. For example: crateAmount = 3 amount = 5 crateBdv = 500 The correct result that it should return is: 5 * 500 / 3 = 833.333 -> 834 (rounded) However, the result of these data would be (5 - 1) * 500 / 3 + 1 = 667 Since the result will be less than expected, the bdv removed to the user's position will be smaller and he will end up with less value deposited than bdv accounted and will be able to mow more stalk than it should.

Impact Details

Since this function is also called during a withdraw of funds, a user could maliciously call several times this function with a specifically crafted amounts to have a bigger BDV accounted but without having deposited an enough amount to back this BDV value. So the end impact will be the user being able to mint more stalk than it should be able to.

References

https://github.com/BeanstalkFarms/Beanstalk/blob/master/protocol/contracts/libraries/Silo/LibTokenSilo.sol#L402

Proof of concept

I contacted with one of the Beanstalk developers (funderbrker) to let them know of the issue as soon as possible and they told me that I should submit it into Immunefi, due to the rush they told me it is not even needed to provide a PoC because it has been already verified.

BIR-20: Micro BDV Stays with Sender

BIC Response

Thank you for this report. Upon review, we have found these findings to be valid. However, given that no additional bdv is issued as a result of this error, and each user's deposit remains in a valid state, with no funds at risk, the BIC has determined the severity of the report is "Medium" with an impact of "Contract fails to deliver promised returns, but doesn't lose value."

As outlined in the program, for Medium severity reports, the BIC determines a reward between 1k and 10k Beans based on:

The exploitability of the bug;
The impact it causes; and
The likelihood of the vulnerability presenting itself.

The BIC has determined this report to be awarded 3,000 Beans.