MohammedRizwan - Uniswap V3 oracles are susceptible to price manipulation on Layer 2 Rollups

[sherlock-admin2]

MohammedRizwan

high

Uniswap V3 oracles are susceptible to price manipulation on Layer 2 Rollups

Summary

Uniswap V3 oracles are susceptible to price manipulation on Layer 2 Rollups

Vulnerability Detail

Per the contest readme.md, The contracts will be deployed on L2 rollups.

On what chains are the smart contracts going to be deployed? mainnet, Arbitrum, Optimism, Base

Therefore, Uniswap official documentation has given an important statement on uniswap v3 oracles integrations on Layer 2 Rollups. This mentions for optimism as below,

Oracles Integrations on Layer 2 Rollups Optimism On Optimism, every transaction is confirmed as an individual block. The block.timestamp of these blocks, however, reflect the block.timestamp of the last L1 block ingested by the Sequencer. For this reason, Uniswap pools on Optimism are not suitable for providing oracle prices, as this high-latency block.timestamp update process makes the oracle much less costly to manipulate. In the future, it's possible that the Optimism block.timestamp will have much higher granularity (with a small trust assumption in the Sequencer), or that forced inclusion transactions will improve oracle security. For more information on these potential upcoming changes, please see the Optimistic Specs repo. For the time being, usage of the oracle feature on Optimism should be avoided.

As the heading of this issue says Layer 2 Rollups. This issue is also apply to Arbitrum as well as Base.

To be noted, Optimism has average block time is 2 seconds and Arbitrum has average block time is 0.26 seconds.

This makes it vulnerable to potential oracle price manipulation.

These TWAP oracle has been used in below contracts,

In Borrower.sol, _getPrices() is used as an internal function to get the prices. Which can be checked here. _getPrices() is further used in getPrices() and getPrices() is further used to fetch the prices from the oracle and used in external functions like warn(), liquidate() and modify(). These functions are at direct risk and the price can be manipulated as stated above.

It is to be noted that uniswap oracle is being used in VolatilityOracle.sol and in oracle.sol but VolatilityOracle.sol under the hood use the unisswap oracle to fetch the functions to be further used in Borrower.sol.

OracleLibrary.consult() can be checked here in OracleLibrary.sol. This is in uniswap pool contract.

Therefore, based on the recommendation by uniswap, It is better to avoid the uniswap twap for L2 rollups.

Impact

The cost of manipulating TWAP in Layer 2 Rollups i.e Optimism, Arbitrum, Base is low, Therefore TWAP oracle should not be used on Optimism, Arbitrum, Base in Aloa project

Code Snippet

https://github.com/sherlock-audit/2023-10-aloe/blob/main/aloe-ii/core/src/Borrower.sol#L479

Tool used

Manual Review

Recommendation

Consider using Chainlink oracles on Layer L2 chains like optimism, Arbitrum, etc.

[sherlock-admin2]

3 comment(s) were left on this issue during the judging contest.

panprog commented:

invalid, because oracle time period is quite large so it can ignore the slower oracle update time, besides protocol has manipulation detection feature

tsvetanovv commented:

Design decision

MohammedRizwan commented:

valid issue with uniswap oracles on l2 rollups as described in report with reference

[0xRizwan]

@haydenshively

Please check this issue. Is this intended design?

[haydenshively]

I believe the Uniswap documentation is out of date. block.timestamp on both Optimism and Arbitrum returns the L2 block time, not the most recent L1 block time, so the latency that Uniswap describes isn't an issue.