Critical Re-entry Vulnerability in UniswapV3OracleHelper's getTimeWeightedTick Function in Oracle.sol

Summary

The function getTimeWeightedTick in the UniswapV3OracleHelper library lacks proper safeguards against re-entry attacks. The absence of such protections may expose the contract to potential vulnerabilities associated with re-entry attacks.

Vulnerability Detail

The vulnerability arises from the fact that the function attempts to fetch data from pool.observe without implementing measures to guard against re-entry attacks. Re-entry attacks occur when a function can be called again before completing its initial execution, potentially leading to unintended and adverse effects on the contract's behavior.

Impact

This vulnerability could be exploited to execute the getTimeWeightedTick function multiple times concurrently, leading to unexpected behavior and potential security risks. Re-entry attacks might result in the manipulation of state variables or the execution of unintended logic, compromising the integrity and security of the contract.

Code Snippet

Oracle.sol https://github.com/sherlock-audit/2023-11-olympus/blob/main/bophades/src/libraries/UniswapV3/Oracle.sol#L67-L92

// ... (relevant code snippets)
function getTimeWeightedTick(address pool_, uint32 period_) public view returns (int56) {
    IUniswapV3Pool pool = IUniswapV3Pool(pool_);
    // ... (other code)

    // Get tick and liquidity from the TWAP
    uint32[] memory observationWindow = new uint32[](2);
    observationWindow[0] = period_;
    observationWindow[1] = 0;

    int56 timeWeightedTick;
    try pool.observe(observationWindow) returns (
        int56[] memory tickCumulatives,
        uint160[] memory
    ) {
        timeWeightedTick = (tickCumulatives[1] - tickCumulatives[0]) / int32(period_);
    } catch (bytes memory) {
        // This function will revert if the observation window is longer than the oldest observation in the pool

        revert UniswapV3OracleHelper_InvalidObservation(pool_, period_);
    }

    // ... (other code)
}
// ... (other code)

Tool used

Manual Review

Recommendation

To enhance the security of the getTimeWeightedTick function and mitigate the risk of re-entry attacks, it is recommended to implement the nonReentrant modifier. Below is an example of how you can modify the function to incorporate this safeguard

// ... (other imports and declarations)

/// @title      UniswapV3OracleHelper
/// @author     0xJem
/// @notice     Helper functions for Uniswap V3 oracles
library UniswapV3OracleHelper {
    // ... (other constants and errors)

    // Define a state variable to track re-entry status
    bool private locked;

    // ... (other functions)

    /// @notice            Determines the time-weighted tick
    /// @dev               This is calculated as the difference between the tick at the end of the period and the tick at the beginning of the period, divided by the period
    ///
    /// @dev               This function will revert if:
    ///                    - The observation window is too short
    ///                    - The observation window is longer than the oldest observation in the pool
    ///                    - The time-weighted tick is outside the bounds of permissible ticks
    ///
    /// @param pool_       The address of the Uniswap V3 pool
    /// @param period_     The period (in seconds) over which to calculate the time-weighted tick
    /// @return            The time-weighted tick
    function getTimeWeightedTick(address pool_, uint32 period_) public view returns (int56) {
        require(!locked, "Re-entry protection: Function locked");

        IUniswapV3Pool pool = IUniswapV3Pool(pool_);

        // Ensure the observation window is long enough
        if (period_ < TWAP_MIN_OBSERVATION_WINDOW)
            revert UniswapV3OracleHelper_ObservationTooShort(
                pool_,
                period_,
                TWAP_MIN_OBSERVATION_WINDOW
            );

        // Get tick and liquidity from the TWAP
        uint32[] memory observationWindow = new uint32[](2);
        observationWindow[0] = period_;
        observationWindow[1] = 0;

        int56 timeWeightedTick;
        try pool.observe(observationWindow) returns (
            int56[] memory tickCumulatives,
            uint160[] memory
        ) {
            timeWeightedTick = (tickCumulatives[1] - tickCumulatives[0]) / int32(period_);
        } catch (bytes memory) {
            // This function will revert if the observation window is longer than the oldest observation in the pool
            // https://github.com/Uniswap/v3-core/blob/d8b1c635c275d2a9450bd6a78f3fa2484fef73eb/contracts/libraries/Oracle.sol#L226C30-L226C30
            revert UniswapV3OracleHelper_InvalidObservation(pool_, period_);
        }

        // Ensure the time-weighted tick is within the bounds of permissible ticks
        // Otherwise getQuoteAtTick will revert: https://docs.uniswap.org/contracts/v3/reference/error-codes
        if (timeWeightedTick > TickMath.MAX_TICK || timeWeightedTick < TickMath.MIN_TICK)
            revert UniswapV3OracleHelper_TickOutOfBounds(
                pool_,
                timeWeightedTick,
                TickMath.MIN_TICK,
                TickMath.MAX_TICK
            );

        // Set the re-entry status to true
        locked = true;

        // Reset the re-entry status after the function completes
        locked = false;

        return timeWeightedTick;
    }

    // ... (other functions)
}

sherlock-audit / 2023-11-olympus-judging

Irissme - Critical Re-entry Vulnerability in UniswapV3OracleHelper's getTimeWeightedTick Function in Oracle.sol #166