Skip to content

Latest commit

 

History

History
105 lines (82 loc) · 4.91 KB

046.md

File metadata and controls

105 lines (82 loc) · 4.91 KB

Perfect Opal Boar

Medium

Vesting Position Protocol Fee Bypass

Summary

The Strategy.sol contract fails to apply protocol fees to assets withdrawn from vesting positions, allowing a significant portion of protocol revenue to bypass fee collection. This occurs because the protocol fee calculation is performed before vesting position withdrawals, rather than after all asset movements are complete.

Snippet

https://github.com/sherlock-audit/2025-02-yieldoor/blob/main/yieldoor/src/Strategy.sol#L159 https://github.com/sherlock-audit/2025-02-yieldoor/blob/main/yieldoor/src/Strategy.sol#L459

Description

The issue stems from the improper sequencing of operations in the collectFees function. Protocol fees are calculated and transferred based on the difference in token balances before and after collecting position fees, but before withdrawing tokens from vesting positions.

function collectFees() public {
    (uint256 preBal0, uint256 preBal1) = idleBalances();

    if (mainPosition.liquidity != 0) collectPositionFees(mainPosition.tickLower, mainPosition.tickUpper);
    if (secondaryPosition.liquidity != 0) {
        collectPositionFees(secondaryPosition.tickLower, secondaryPosition.tickUpper);
    }
    if (ongoingVestingPosition) {
        collectPositionFees(vestPosition.tickLower, mainPosition.tickUpper);
    }

    (uint256 afterBal0, uint256 afterBal1) = idleBalances();

    uint256 protocolFees0 = (afterBal0 - preBal0) * protocolFee / 10_000;
    uint256 protocolFees1 = (afterBal1 - preBal1) * protocolFee / 10_000;

    if (protocolFees0 > 0) IERC20(token0).safeTransfer(feeRecipient, protocolFees0);
    if (protocolFees1 > 0) IERC20(token1).safeTransfer(feeRecipient, protocolFees1);

    if (ongoingVestingPosition) {
        _withdrawPartOfVestingPosition();
    }
}

The _withdrawPartOfVestingPosition function removes liquidity from the vesting position based on time elapsed.

function _withdrawPartOfVestingPosition() internal {
    if (vestPosition.lastUpdate == block.timestamp) return;
    VestingPosition memory vp = vestPosition;

    uint256 lastValid = block.timestamp < vp.endTs ? block.timestamp : vp.endTs;

    uint128 liquidityToRemove = lastValid == vp.endTs
        ? vp.remainingLiquidity
        : uint128(vp.initialLiquidity * (lastValid - vp.lastUpdate) / (vp.endTs - vp.startTs));

    _removeFromPosition(liquidityToRemove, vp.tickLower, vp.tickUpper);

    vp.remainingLiquidity -= liquidityToRemove;
    vp.lastUpdate = lastValid;

    vestPosition = vp;

    if (lastValid == vestPosition.endTs) ongoingVestingPosition = false;
}

This function withdraws tokens from the vesting position but does not apply any protocol fee to these withdrawn tokens.

Scenario

  1. A vesting position is created with 10 ETH and 20,000 USDC via addVestingPosition with a 30-day vesting period
  2. The protocol fee is set to 10% (1000 basis points)
  3. After 15 days (50% of vesting period), collectFees is called
  4. The function collects trading fees from all positions (including vesting)
  5. Protocol fees are calculated and transferred based only on these collected trading fees
  6. _withdrawPartOfVestingPosition is called, which withdraws 5 ETH and 10,000 USDC (50% of the vesting position)
  7. These withdrawn tokens (worth approximately $25,000 at current prices) are not subject to the protocol fee
  8. The protocol treasury loses 0.5 ETH and 1,000 USDC (approximately $2,500) in uncollected fees

Impact

For protocols with significant vesting incentives, this can result in significant revenue losses over time. Additionally, it creates an economic incentive to use vesting positions rather than regular positions to avoid protocol fees, thereby distorting the protocol’s economic model.

Recommendation

Restructure the collectFees function to calculate protocol fees after all asset movements, including vesting position withdrawals.

function collectFees() public {
    (uint256 preBal0, uint256 preBal1) = idleBalances();

    // Collect fees from all positions
    if (mainPosition.liquidity != 0) collectPositionFees(mainPosition.tickLower, mainPosition.tickUpper);
    if (secondaryPosition.liquidity != 0) {
        collectPositionFees(secondaryPosition.tickLower, secondaryPosition.tickUpper);
    }
    if (ongoingVestingPosition) {
        collectPositionFees(vestPosition.tickLower, vestPosition.tickUpper);
        // Withdraw vesting position BEFORE calculating protocol fees
        _withdrawPartOfVestingPosition();
    }

    // Calculate protocol fees AFTER all asset movements
    (uint256 afterBal0, uint256 afterBal1) = idleBalances();
    
    uint256 protocolFees0 = (afterBal0 - preBal0) * protocolFee / 10_000;
    uint256 protocolFees1 = (afterBal1 - preBal1) * protocolFee / 10_000;

    // Transfer protocol fees
    if (protocolFees0 > 0) IERC20(token0).safeTransfer(feeRecipient, protocolFees0);
    if (protocolFees1 > 0) IERC20(token1).safeTransfer(feeRecipient, protocolFees1);
}