Uniswap v2 price determined based on reserves can be manipulated

[hats-bug-reporter[bot]]

Github username: @ahmaddecoded Submission hash (on-chain): 0x6f6db8667ad9cff386f5dd3e9195c3b2a46f55c6eb4b905c619d8bd9d19219d6 Severity: high

Description: Description\ Uniswap v2 price determined based on reserves can be manipulated Attack Scenario\ Price calculated based upon reserves is easily manipulatable using the flash loan and has been cause of critical vulnerabilities in past Attachments

1. Proof of Concept (PoC) File

   function _getV2Price(
       address uniswapV2Pool,
       bool isReversed,
       bool isEthPriceRelated
   ) internal view returns (uint256, bool) {
       (uint112 reserve0, uint112 reserve1, ) = IUniswapV2Pair(uniswapV2Pool).getReserves();
       uint256 price = reserve0 < reserve1
           ? (reserve1 * 10 ** _getDecimalsDelta(uniswapV2Pool)) / reserve0
           : (reserve0 * 10 ** _getDecimalsDelta(uniswapV2Pool)) / reserve1;
       return _postTreatmentAndVerifyEth(price, isReversed, isEthPriceRelated);
   }

Assets can be easily changed in one transaction using flashloan and than performing an operation on convergence. That could lead to substantial loss.

Instead use the TWAp price recommended by uniswap

2. Revised Code File (Optional)

[walk-on-me]

Hello, Thanks a lot for your attention.

Your assumption is right if we didn't have the deltaAggregator parameter. This parameter compare the price returned by the LP with the Chainlink one. We'll use low % in production < 1-2%, this paired with deep liquidity LP will prevent Liquidity attacks.

This is the design we choose,

Because of this, we have to consider this issue as Invalid