Skip to content

Latest commit

 

History

History
172 lines (128 loc) · 5.67 KB

061.md

File metadata and controls

172 lines (128 loc) · 5.67 KB

Clever Burgundy Poodle

Medium

Division by zero in checkPoolActivity() will cause revert in rebalance() for newly initialized pools

Summary

The missing check for identical timestamps in Strategy.sol will cause a complete denial of service for users as the contract will revert when attempting to rebalance or add vesting positions on newly initialized pools with only one observation.

Root Cause

In Strategy.sol:checkPoolActivity():326 there is a division by zero vulnerability when calculating the average tick. When a pool has only one observation (which is the case for newly initialized pools), nextTimestamp will equal timestamp, causing division by zero.

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

Internal Pre-conditions

N/A

External Pre-conditions

  1. The Uniswap V3 pool must have an observationCardinality of 1, which is the default for newly initialized pools1.
  2. No swaps or liquidity events have occurred on the pool since initialization that increased the observation amount

Attack Path

  1. A new Uniswap V3 pool is created and initialized
  2. The Strategy contract is deployed and configured to use this newly initialized pool
  3. Rebalancer attempts to call a function that triggers checkPoolActivity(), such as rebalance()
  4. The function retrieves the current observation at index 0 and attempts to get the next observation, which is also at index 0
  5. Both observations have identical timestamps since there's only one observation
  6. When calculating the average tick, the code attempts to divide by zero: (nextTimestamp - timestamp) equals 0
  7. The transaction reverts due to division by zero, preventing any rebalancing or adding of vesting positions

Impact

The protocol suffers a temporary denial of service for critical functionality. Rebalancer cannot rebalance positions or owner cannot add vesting positions on newly initialized pools

PoC

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import "forge-std/Test.sol";
import {Vault} from "../src/Vault.sol";
import {IVault} from "../src/interfaces/IVault.sol";
import {IStrategy} from "../src/interfaces/IStrategy.sol";
import {IUniswapV3Pool} from "../src/interfaces/IUniswapV3Pool.sol";
import {IERC20} from "@openzeppelin/token/ERC20/IERC20.sol";
import {Strategy} from "../src/Strategy.sol";
import {IMainnetRouter} from "../src/interfaces/IMainnetRouter.sol";
import {LiquidityAmounts} from "../src/libraries/LiquidityAmounts.sol";
import {TickMath} from "../src/libraries/TickMath.sol";
import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";

struct MintParams {
    address token0;
    address token1;
    uint24 fee;
    int24 tickLower;
    int24 tickUpper;
    uint256 amount0Desired;
    uint256 amount1Desired;
    uint256 amount0Min;
    uint256 amount1Min;
    address recipient;
    uint256 deadline;
}

interface INonfungiblePositionManager {
    function mint(MintParams calldata params)
        external
        payable
        returns (uint256 tokenId, uint128 liquidity, uint256 amount0, uint256 amount1);

    function createAndInitializePoolIfNecessary(address token0, address token1, uint24 fee, uint160 sqrtPriceX96)
        external
        payable
        returns (address pool);
}

contract Token is ERC20 {
    constructor() ERC20("Test Token", "TT") {
        _mint(msg.sender, 1000000 * 10 ** 18);
    }
}

contract NoObservations is Test {
    INonfungiblePositionManager nonfungiblePositionManager =
        INonfungiblePositionManager(0xC36442b4a4522E871399CD717aBDD847Ab11FE88);
    address uniRouter = address(0xE592427A0AEce92De3Edee1F18E0157C05861564);

    Token token0;
    Token token1;
    IUniswapV3Pool pool;
    address vault;
    address strategy;
    address rebalancer = address(1001);
    address feeRecipient = address(9999);
    address user = address(1);
    address depositor = address(100000000001);
    address owner = address(222);

    function setUp() public virtual {
        vm.startPrank(owner);

        token0 = new Token();
        token1 = new Token();

        if (address(token0) > address(token1)) {
            (token0, token1) = (token1, token0);
        }

        uint160 sqrtPriceX96 = 2 ** 96;
        uint24 fee = 100; // 0.1%
        pool = IUniswapV3Pool(
            nonfungiblePositionManager.createAndInitializePoolIfNecessary(
                address(token0), address(token1), fee, sqrtPriceX96
            )
        );

        token0.approve(address(pool), type(uint256).max);
        token1.approve(address(pool), type(uint256).max);

        vault = address(new Vault(address(token0), address(token1)));
        strategy = address(new Strategy(address(pool), address(vault), rebalancer, feeRecipient));
        IVault(vault).setStrategy(strategy);

        vm.stopPrank();
    }

    function test_noObservations() public {
        vm.startPrank(owner);

        // deposit to vault
        token0.approve(vault, type(uint256).max);
        token1.approve(vault, type(uint256).max);

        IVault(vault).deposit(100e18, 100e18, 0, 0);

        skip(10 minutes);

        // rebalance
        vm.startPrank(rebalancer);

        vm.expectRevert(stdError.divisionError);
        IStrategy(strategy).rebalance();
    }
}

Mitigation

Add a check to handle the case when timestamps are identical:

function checkPoolActivity() public view returns (bool) {
    // ... existing code ...
    
    // Check if timestamps are identical (single observation case)
    if (nextTimestamp == timestamp) {
        return true;
    }

    tick = int24((nextCumulativeTick - tickCumulative) / int56(uint56(nextTimestamp - timestamp)));
    
    // ... rest of the function ...
}