claimFees() allows faulty fee claims

[c4-bot-1]

Lines of code

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

Vulnerability details

Impact

Fee claimers can get a suboptimal payout, or claims can be reverted

Description

Users can call claimFees() on factory owner for a specific pool by passing the requested amounts and paying the payout amount.

Manually passing the requested token amounts may lead suboptimal results causing the fee claimer to get less then they should get or reverting the tx altogether if they ask for the actual maximum amount because of the gas savings on the pool contract.

Proof of Concept

protocolFees.token0 = 1e18 protocolFees.token1 = 2e18

Scenario 1: User calls claimFees with following params because of a calculation mistake _amount0Requested = 1e17 _amount1Requested = 2e17 User receives less than they should get and looses value

Scenario 2: User calls claimFees with max of the fee amount collected _amount0Requested = 1e18 _amount1Requested = 2e18 Because of the gas savings here: https://github.com/Uniswap/v3-core/blob/d8b1c635c275d2a9450bd6a78f3fa2484fef73eb/contracts/UniswapV3Pool.sol#L857 Although the call was valid, claim reverts with V3FactoryOwner__InsufficientFeesCollected

Scenario 3: User calls claimFees with max of the fee amount collected - 1 _amount0Requested = 1e18 - 1 _amount1Requested = 2e18 - 1 And if some large swap happens while the claim was in the mempool, the user will leave some fees in the pool

Tools Used

Manual review

Recommended Mitigation Steps

• Change claimFees() to accept _minAmount0 and _minAmount1 instead of requested amounts.
• claimFees() makes a call to the pool to get the current fee amounts accrued in the pool
• claimFees calls collectProtocol() with feeAmounts - 1
• require returned fees from the pool is GTE to min amounts

Assessed type

Math

[c4-judge]

MarioPoneder marked the issue as duplicate of #34

[c4-judge]

MarioPoneder marked the issue as satisfactory

[c4-judge]

MarioPoneder marked the issue as grade-b