FloorDAO / floor-v2

Floor aims to create a fully onchain governance mechanism for sweeping and deploying NFTs to profitable NFT-Fi strategies as well as seeding liquidity for its own NFT-Fi products.
https://floor.xyz
2 stars 0 forks source link

[UVP-04M] Inexistent Mandation of Minimum Observation Period #108

Closed tomwade closed 1 year ago

tomwade commented 1 year ago

UVP-04M: Inexistent Mandation of Minimum Observation Period

Type Severity Location
Logical Fault UniswapV3PricingExecutor.sol:L185-L186

Description:

The UniswapV3PricingExecutor::_getPrice function does not mandate a minimum ago threshold when the initial observation fails, permitting a potential ago value of 0 to be passed in to the observe call rendering the price measurement insecure.

Impact:

In a pair that has just been created, it is possible to arbitrarily manipulate the price measurement reported by the system.

Example:

// Set our default TWAP ago time
uint ago = 1800;

// We set our TWAP range to 30 minutes
uint32[] memory secondsAgos = new uint32[](2);
(secondsAgos[0], secondsAgos[1]) = (uint32(ago), 0);

// If we cannot find an observation for our desired time, then we attempt to fallback
// on the latest observation available to us
(bool success, bytes memory data) = pool.staticcall(abi.encodeWithSelector(IUniswapV3Pool.observe.selector, secondsAgos));
if (!success) {
    if (keccak256(data) != keccak256(abi.encodeWithSignature('Error(string)', 'OLD'))) revertBytes(data);

    // The oldest available observation in the ring buffer is the index following the current (accounting for wrapping),
    // since this is the one that will be overwritten next.
    (,, uint16 index, uint16 cardinality,,,) = IUniswapV3Pool(pool).slot0();
    (uint32 oldestAvailableAge,,, bool initialized) = IUniswapV3Pool(pool).observations((index + 1) % cardinality);

    // If the following observation in a ring buffer of our current cardinality is uninitialized, then all the
    // observations at higher indices are also uninitialized, so we wrap back to index 0, which we now know
    // to be the oldest available observation.
    if (!initialized) (oldestAvailableAge,,,) = IUniswapV3Pool(pool).observations(0);

    // Update our "ago" seconds to the value of the latest observation
    ago = block.timestamp - oldestAvailableAge;
    secondsAgos[0] = uint32(ago);

    // Call observe() again to get the oldest available
    (success, data) = pool.staticcall(abi.encodeWithSelector(IUniswapV3Pool.observe.selector, secondsAgos));
    if (!success) revertBytes(data);
}

Recommendation:

We advise the code to ensure a minimum ago value in the observation failure case to ensure that the price manipulation cost is prohibitively expensive.

tomwade commented 1 year ago

I have set this to require a minimum of 5 minutes. This can be configured based on recommendation