Losses to MEV searchers due to fake uniswap pools

[c4-bot-6]

Lines of code

https://github.com/code-423n4/2024-02-uniswap-foundation/blob/5a2761c8277541a24bc551fbd624413b384bea94/src/V3FactoryOwner.sol#L189-L195

Vulnerability details

Impact

Honest MEV searchers can be tricked and may lose their money. This can lead to them going insolvent and not being able to claimFees again.

Proof of Concept

It is expected that V3FactoryOwner.claimFees() will be used by MEV searchers to use their funds to get more WETH. MEV bots often try to offer more money for transaction fees to get their transactions included in the next block before others. This situation creates a chance for dishonest MEV bots to make fake Uniswap pools, misleading honest MEV searchers into executing claimFees on those pools, only to find out they receive no rewards.

Here's how it happens:

Imagine two MEV searchers competing: A, who is malicious, and B.

1. A sets up a simple fake Uniswap pool with just a collectProtocol function, pretending to have the return values needed to pass the if (_amount0 < _amount0Requested || _amount1 < _amount1Requested) check. Example implementation:

   contract FakePool is IUniswapPool {
   function collectProtocol(
       address recipient,
       uint128 amount0Requested,
       uint128 amount1Requested
   ) external returns (uint128 amount0, uint128 amount1) {
       amount0 = amount0Requested;
       amount1 = amount1Requested;
   }
   }

2. A uses V3FactoryOwner.claimFees(), passing the fake pool, hoping to entice others to attempt to frontrun this transaction.
3. B sees A's tempting transaction and tries to get ahead by submitting a similar transaction with a higher transaction fee.
4. Although the WETH tokens are transferred to the UniStaker and allocated among stakers, B ends up without any reward.

Tool Usage

Manual Review

Recommended Mitigation Steps

It is suggested to change the claimFees function to accept tokenA, tokenB, and fee as inputs, and to internally derive the pool address from the UniswapV3Factory.

  function claimFees(
-   IUniswapV3PoolOwnerActions _pool,
+   address tokenA,
+   address tokenB,
+   uint24 fee,
    address _recipient,
    uint128 _amount0Requested,
    uint128 _amount1Requested
  ) external returns (uint128, uint128) {
+   IUniswapV3PoolOwnerActions _pool = FACTORY.getPool(tokenA, tokenB, fee);

    PAYOUT_TOKEN.safeTransferFrom(msg.sender, address(REWARD_RECEIVER), payoutAmount);
    REWARD_RECEIVER.notifyRewardAmount(payoutAmount);
    (uint128 _amount0, uint128 _amount1) =
      _pool.collectProtocol(_recipient, _amount0Requested, _amount1Requested);

    // Ensure the caller gets no less than they asked for. See `collectProtocol` for context.
    if (_amount0 < _amount0Requested || _amount1 < _amount1Requested) {
      revert V3FactoryOwner__InsufficientFeesCollected();
    }
    emit FeesClaimed(address(_pool), msg.sender, _recipient, _amount0, _amount1);
    return (_amount0, _amount1);
  }

Assessed type

MEV

[c4-judge]

MarioPoneder marked the issue as duplicate of #380

[c4-judge]

MarioPoneder changed the severity to QA (Quality Assurance)

[c4-judge]

MarioPoneder marked the issue as grade-b