code-423n4 / 2021-05-yield-findings

0 stars 0 forks source link

Uniswap Oracle uses wrong prices #26

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

cmichel

Vulnerability details

The Uniswap oracle uses a mock contract with hard-coded prices to retrieve the price which is not feasible in production. Not sure if this is part of the contest, this will probably still be changed? But note that even when using the "real deal" @uniswap/v3-periphery/contracts/libraries/OracleLibrary.sol it does not return the prices.

Impact

The price could change from the set price and always updating new prices with set will be too slow and gas expensive.

Recommended Mitigation Steps

Use cumulativeTicks = pool.observe([secondsAgo, 0]) // [a_t1, a_t2] and apply equation 5.5 from the Uniswap V3 whitepaper to compute the token0 TWAP. Note that even the official .consult call seems to only return the averaged cumulative ticks, you'd still need to compute the 1.0001^timeWeightedAverageTick in the function.

alcueca commented 3 years ago

We probably should have not included this contract, it's too confusing since at the time the Uniswap v3 OracleLibrary was still a mock, and this hasn't gone real testing.

The price source in a production version would be a Uniswap v3 pool, not one of our mock oracle sources. We never expected to call set in production, but to retrieve the prices from a Uniswap v3 pool using the mentioned library (which was not even merged into main at the start of the contest).

We will check with the Uniswap team what is the recommended way of using their oracles. The equation 5.5 in the whitepaper is problematic, because an exponentiation of two fractional numbers in solidity is neither trivial nor cheap. Our understanding is that one of the goals of the OracleLibrary was to provide a consistent implementation to this formula.

From a conversation with @moodysalem, I understand that the code in getQuoteAtTick might achieve the same result as the 5.5 equation, so maybe we need to retrieve the average tick with consult, and then the actual price with getQuoteAtTick.

alcueca commented 3 years ago

I'm using the acknowledged label for findings that require further investigation to assess.