Scrawny Ivory Badger - `DepositQueue.cancelDepositRequest()` corrupts the Fenwick Tree due to incorrect index usage

[sherlock-admin2]

Scrawny Ivory Badger

Medium

DepositQueue.cancelDepositRequest() corrupts the Fenwick Tree due to incorrect index usage

Summary

DepositQueue.cancelDepositRequest() subtracts the user's deposit amount from an incorrect bucket in the Fenwick tree, as it subtracts from the latest checkpoint bucket instead of the original bucket where the deposit was made.

This causes a desynchronization in the Fenwick prefix sums, resulting in incorrect total asset accounting, eventually leading to incorrect share minting.

Root Cause

https://github.com/sherlock-audit/2025-07-mellow-flexible-vaults/blob/3135edc3bae494ca1a5427938158bad0d28dff57/flexible-vaults/src/queues/DepositQueue.sol#L98-L117

function cancelDepositRequest() external nonReentrant {
        address caller = _msgSender();
        DepositQueueStorage storage $ = _depositQueueStorage();
        Checkpoints.Checkpoint224 memory request = $.requestOf[caller];
        uint256 assets = request._value;
        if (assets == 0) {
            revert NoPendingRequest();
        }
        address asset_ = asset();
        (bool exists, uint32 timestamp, uint256 index) = $.prices.latestCheckpoint(); //@audit - latest bucket
        if (exists && timestamp >= request._key) {
            revert ClaimableRequestExists();
        }

        delete $.requestOf[caller];
        IVaultModule(vault()).riskManager().modifyPendingAssets(asset_, -int256(uint256(assets)));
        $.requests.modify(index, -int256(assets)); //@audit - here
        TransferLibrary.sendAssets(asset_, caller, assets);
        emit DepositRequestCanceled(caller, assets, request._key);
    }

The code assumes that the latest price checkpoint index is the same as the user's original deposit bucket.

But these trees are not guaranteed to be synchronized: a user’s deposit may be in a much earlier bucket.

The tree is corrupted: the wrong bucket is decremented, while the correct bucket still reflects the full deposit amout

Internal Pre-conditions

1-A user has a pending deposit stored in a bucket i > 0

One of the following is true:

2-No price checkpoint exists (exists == false, so index = 0)
2-A checkpoint exists in an earlier bucket than the deposit (index < i)

3-The user cancels the deposit before a matching price report occurs

External Pre-conditions

None

Attack Path

1.Alice deposits 1 ETH at timestamp T, request stored in bucket 0
2.Bob deposits 1 ETH at T + 1 hour, request stored in bucket 1
3.A price report is pushed (at T + 1.5 hours). sets the latest checkpoint to bucket 1
4.Alice cancels her deposit, request in bucket 0 but execution is bucket 1 due to the bug.
5.So bucket 1 is reduced, but bucket 0 still holds +1 ETH
6.Bob tries to cancel his deposit but cant

Impact

State corruption in the Fenwick tree (incorrect prefix sums), resulting in eventually dilution of honest users by minting unbacked shares, or simply DOS of cancellation of deposit (see attack path)

PoC

N/A

Mitigation

Modify cancelDepositRequest() to track the original request bucket index rather than relying on latestCheckpoint()