041.md

Perfect Opal Boar

High

Incorrect Tick Range in Vesting Position Fee Collection

Summary

The Strategy contract contains a critical issue in the collectFees function where fees from vesting positions are collected using an incorrect tick range. The function uses the lower tick from the vesting position but incorrectly uses the upper tick from the main position. This mismatch creates a "hybrid position" that likely doesn't exist in the Uniswap V3 pool, resulting in uncollected fees that remain permanently locked in the pool contract.

Snippet

https://github.com/sherlock-audit/2025-02-yieldoor/blob/main/yieldoor/src/Strategy.sol#L166-L168 https://github.com/sherlock-audit/2025-02-yieldoor/blob/main/yieldoor/src/Strategy.sol#L433-L434 https://github.com/sherlock-audit/2025-02-yieldoor/blob/main/yieldoor/src/Strategy.sol#L209

Description

The root cause of this vulnerability is in the collectFees function of the Strategy contract, where an incorrect parameter is passed to the collectPositionFees function.

function collectFees() public {
    // ... Other code ...

    if (ongoingVestingPosition) {
        collectPositionFees(vestPosition.tickLower, mainPosition.tickUpper);
    }

    // ... Other code ...
}

The function incorrectly uses mainPosition.tickUpper instead of vestPosition.tickUpper as the second parameter. This creates a mismatch between the actual vesting position in the Uniswap V3 pool and the position from which the contract attempts to collect fees.

The issue is particularly problematic because the vesting position is initially created with the same tick range as the main position.

function addVestingPosition(uint256 amount0, uint256 amount1, uint256 _vestDuration) external onlyVault {
    // ... Other code ...

    vp.tickLower = mainPosition.tickLower;
    vp.tickUpper = mainPosition.tickUpper;

    // ... Other code ...
}

However, the main position's tick range can change after a vesting position is created, particularly during rebalancing operations.

function rebalance() public onlyRebalancer {
    // ...
    _setMainTicks(tick);
    // ...
}

When this happens, mainPosition.tickUpper will no longer match vestPosition.tickUpper, causing the fee collection to target a non-existent position.

Scenario

1. A vesting position is created via addVestingPosition, with the same tick range as the main position.
2. The main position is rebalanced via rebalance, changing mainPosition.tickLower and mainPosition.tickUpper.
3. The collectFees function is called, either directly or as part of another operation.
4. The function attempts to collect fees using vestPosition.tickLower and `mainPosition.tickUpper.
5. Since this position doesn't exist in the Uniswap V3 pool, no fees are collected.
6. The fees generated by the actual vesting position (defined by vestPosition.tickLower and vestPosition.tickUpper) remain locked in the Uniswap V3 pool contract.

Impact

Fees generated by vesting positions become permanently locked in the Uniswap V3 pool contract and cannot be collected by the Strategy contract.

Recommendation

Replace the incorrect parameter in the collectFees function.

if (ongoingVestingPosition) {
    collectPositionFees(vestPosition.tickLower, vestPosition.tickUpper);
}