Audit Fixes: Uniswap V2 DoS

script/salts/salts.json

@@ -102,13 +102,13 @@
     "0xdb5689fd93b97ad35ee469bbcb38d4472c005f1ae951b154f2437bd1207a03f3": "0xf35c7565f04f89db7d1783343067728c4db35f70d7aea4d7dbd732fa462bb956"
   },
   "Test_UniswapV2DirectToLiquidity": {
-    "0x37d0a777ee76878951a8fdcfee36511eef6c971d5e3eb994cdfc8c0141701fe6": "0x35b1fb1a02bed036ef5072d33d984c29d8e5cf533dfa3bd3a22fed820b4ad620"
+    "0xea7257f131ae4e2b30b474b28eccadd1f8831afb3f83eb83983538ca3410e64b": "0xc5a7d043582a4ea36726c078f1fe927f32e04868176654c04103bc51f486a74f"
   },
   "Test_UniswapV2Router": {
     "0x3aeb5c743a058e3c9d871533d2537013b819c4e401acaca3619f046cd9422258": "0x4c096423447dffd4ffce23f8e15e2d1b08619f72abc81536b5492d95b7c8f03f"
   },
   "Test_UniswapV3DirectToLiquidity": {
-    "0x10605d8e49d2673f5714dc41ae45c9e14e608e378417d144738a7896048e21d5": "0xa879225ae7704a9c5cfa3c470fd724a7ae4d14e8b159745b9af0938710694eee"
+    "0x10605d8e49d2673f5714dc41ae45c9e14e608e378417d144738a7896048e21d5": "0x76ca8e30d84c41d94ba233af2fbc1322e8353b87f36cee2dd2ecd378e67014ec"
   },
   "Test_UniswapV3Factory": {
     "0x33479a3955b5801da918af0aa61f4501368d483ac02e8ae036d4e6f9c56d2038": "0x715638753ed78c13d0d385c1758204daad3b308083604fb0d555eb285199ed30"

src/callbacks/liquidity/UniswapV2DTL.sol

@@ -3,9 +3,11 @@ pragma solidity ^0.8.19;
 
 // Libraries
 import {ERC20} from "@solmate-6.7.0/tokens/ERC20.sol";
+import {FullMath} from "@uniswap-v3-core-1.0.1-solc-0.8-simulate/libraries/FullMath.sol";
 
 // Uniswap
 import {IUniswapV2Factory} from "@uniswap-v2-core-1.0.1/interfaces/IUniswapV2Factory.sol";
+import {IUniswapV2Pair} from "@uniswap-v2-core-1.0.1/interfaces/IUniswapV2Pair.sol";
 import {IUniswapV2Router02} from "@uniswap-v2-periphery-1.0.1/interfaces/IUniswapV2Router02.sol";
 
 // Callbacks
@@ -95,6 +97,7 @@ contract UniswapV2DirectToLiquidity is BaseDirectToLiquidity {
     /// @inheritdoc BaseDirectToLiquidity
     /// @dev        This function implements the following:
     ///             - Creates the pool if necessary
+    ///             - Detects and handles (if necessary) manipulation of pool reserves
     ///             - Deposits the tokens into the pool
     function _mintAndDeposit(
         uint96 lotId_,
@@ -114,23 +117,44 @@ contract UniswapV2DirectToLiquidity is BaseDirectToLiquidity {
             pairAddress = uniV2Factory.createPair(baseToken_, quoteToken_);
         }
 
+        // Handle a potential DoS attack caused by donate and sync
+        uint256 quoteTokensToAdd = quoteTokenAmount_;
+        uint256 baseTokensToAdd = baseTokenAmount_;
+        {
+            uint256 auctionPrice = FullMath.mulDiv(
+                quoteTokenAmount_, 10 ** ERC20(baseToken_).decimals(), baseTokenAmount_
+            );
+
+            (, uint256 baseTokensUsed) =
+                _mitigateDonation(pairAddress, auctionPrice, quoteToken_, baseToken_);
+
+            if (baseTokensUsed > 0) {
+                baseTokensToAdd -= baseTokensUsed;
+
+                // Re-calculate quoteTokensToAdd to be aligned with baseTokensToAdd
+                quoteTokensToAdd = FullMath.mulDiv(
+                    baseTokensToAdd, auctionPrice, 10 ** ERC20(baseToken_).decimals()
+                );
+            }
+        }
+
         // Calculate the minimum amount out for each token
-        uint256 quoteTokenAmountMin = _getAmountWithSlippage(quoteTokenAmount_, params.maxSlippage);
-        uint256 baseTokenAmountMin = _getAmountWithSlippage(baseTokenAmount_, params.maxSlippage);
+        uint256 quoteTokenAmountMin = _getAmountWithSlippage(quoteTokensToAdd, params.maxSlippage);
+        uint256 baseTokenAmountMin = _getAmountWithSlippage(baseTokensToAdd, params.maxSlippage);
 
         // Approve the router to spend the tokens
-        ERC20(quoteToken_).approve(address(uniV2Router), quoteTokenAmount_);
-        ERC20(baseToken_).approve(address(uniV2Router), baseTokenAmount_);
+        ERC20(quoteToken_).approve(address(uniV2Router), quoteTokensToAdd);
+        ERC20(baseToken_).approve(address(uniV2Router), baseTokensToAdd);
 
         // Deposit into the pool
         uniV2Router.addLiquidity(
             quoteToken_,
             baseToken_,
-            quoteTokenAmount_,
-            baseTokenAmount_,
+            quoteTokensToAdd,
+            baseTokensToAdd,
             quoteTokenAmountMin,
             baseTokenAmountMin,
-            address(this),
+            address(this), // LP tokens are sent to this contract and transferred later
             block.timestamp
         );
 
@@ -157,4 +181,96 @@ contract UniswapV2DirectToLiquidity is BaseDirectToLiquidity {
 
         return abi.decode(lotConfig.implParams, (UniswapV2OnCreateParams));
     }
+
+    /// @notice This function mitigates the risk of a third-party having donated quote tokens to the pool causing the auction settlement to fail.
+    /// @dev    It performs the following:
+    ///         - Checks if the pool has had quote tokens donated, or exits
+    ///         - Swaps the quote tokens for base tokens to adjust the reserves to the correct price
+    ///
+    /// @param  pairAddress_    The address of the Uniswap V2 pair
+    /// @param  auctionPrice_   The price of the auction
+    /// @param  quoteToken_     The quote token of the pair
+    /// @param  baseToken_      The base token of the pair
+    /// @return quoteTokensUsed The amount of quote tokens used in the swap
+    /// @return baseTokensUsed  The amount of base tokens used in the swap
+    function _mitigateDonation(
+        address pairAddress_,
+        uint256 auctionPrice_,
+        address quoteToken_,
+        address baseToken_
+    ) internal returns (uint256 quoteTokensUsed, uint256 baseTokensUsed) {
+        IUniswapV2Pair pair = IUniswapV2Pair(pairAddress_);
+        uint256 quoteTokenBalance = ERC20(quoteToken_).balanceOf(pairAddress_);
+        {
+            // Check if the pool has had quote tokens donated (whether synced or not)
+            // Base tokens are not liquid, so we don't need to check for them
+            (uint112 reserve0, uint112 reserve1,) = pair.getReserves();
+            uint112 quoteTokenReserve = pair.token0() == quoteToken_ ? reserve0 : reserve1;
+
+            if (quoteTokenReserve == 0 && quoteTokenBalance == 0) {
+                return (0, 0);
+            }
+        }
+
+        // If there has been a donation into the pool, we need to adjust the reserves so that the price is correct
+        // This can be performed by swapping the quote tokens for base tokens
+
+        // To perform the swap, both reserves need to be non-zero, so we need to transfer in some base tokens and update the reserves using `sync()`.
+        {
+            ERC20(baseToken_).transfer(pairAddress_, 1);
+            pair.sync();
+            baseTokensUsed += 1;
+        }
+
+        // We want the pool to end up at the auction price
+        // The simplest way to do this is to have the auctionPrice_ of quote tokens
+        // and 1 of base tokens in the pool (accounting for decimals)
+        uint256 desiredQuoteTokenReserves = auctionPrice_;
+        uint256 desiredBaseTokenReserves = 10 ** ERC20(baseToken_).decimals();
+
+        // Handle quote token transfers
+        uint256 quoteTokensOut;
+        {
+            // If the balance is less than required, transfer in
+            if (quoteTokenBalance < desiredQuoteTokenReserves) {
+                uint256 quoteTokensToTransfer = desiredQuoteTokenReserves - quoteTokenBalance;
+                ERC20(quoteToken_).transfer(pairAddress_, quoteTokensToTransfer);
+
+                quoteTokensUsed += quoteTokensToTransfer;
+
+                // Update the balance
+                quoteTokenBalance = desiredQuoteTokenReserves;
+            }
+
+            // Determine the amount of quote tokens to swap out
+            quoteTokensOut = quoteTokenBalance - desiredQuoteTokenReserves;
+        }
+
+        // Handle base token transfers
+        {
+            uint256 baseTokensToTransfer =
+                desiredBaseTokenReserves - ERC20(baseToken_).balanceOf(pairAddress_);
+            if (baseTokensToTransfer > 0) {
+                ERC20(baseToken_).transfer(pairAddress_, baseTokensToTransfer);
+                baseTokensUsed += baseTokensToTransfer;
+            }
+        }
+
+        // Perform the swap
+        uint256 amount0Out = pair.token0() == quoteToken_ ? quoteTokensOut : 0;
+        uint256 amount1Out = pair.token0() == quoteToken_ ? 0 : quoteTokensOut;
+
+        if (amount0Out > 0 || amount1Out > 0) {
+            pair.swap(amount0Out, amount1Out, address(this), "");
+        } else {
+            // If no swap is needed, sync the pair to update the reserves
+            pair.sync();
+        }
+
+        // The pool reserves should now indicate the correct price.
+        // This contract may now hold additional quote tokens that were transferred from the pool.
+        // These tokens will be transferred to the seller during cleanup.
+
+        return (quoteTokensUsed, baseTokensUsed);
+    }
 }

test/callbacks/liquidity/UniswapV2DTL/UniswapV2DTLTest.sol

@@ -38,6 +38,7 @@ abstract contract UniswapV2DirectToLiquidityTest is Test, Permit2User, WithSalts
     address internal constant _NOT_SELLER = address(0x20);
 
     uint96 internal constant _LOT_CAPACITY = 10e18;
+    uint24 internal constant _MAX_SLIPPAGE = 1; // 0.01%
 
     uint48 internal constant _START = 1_000_000;
     uint48 internal constant _DURATION = 1 days;
@@ -57,12 +58,15 @@ abstract contract UniswapV2DirectToLiquidityTest is Test, Permit2User, WithSalts
     MockERC20 internal _quoteToken;
     MockERC20 internal _baseToken;
 
+    uint96 internal _lotCapacity = _LOT_CAPACITY;
     uint96 internal _proceeds;
     uint96 internal _refund;
 
+    // TODO consider setting floor of max slippage to 0.01%
+
     // Inputs
     UniswapV2DirectToLiquidity.UniswapV2OnCreateParams internal _uniswapV2CreateParams =
-        UniswapV2DirectToLiquidity.UniswapV2OnCreateParams({maxSlippage: uint24(0)});
+        UniswapV2DirectToLiquidity.UniswapV2OnCreateParams({maxSlippage: uint24(_MAX_SLIPPAGE)});
     BaseDirectToLiquidity.OnCreateParams internal _dtlCreateParams = BaseDirectToLiquidity
         .OnCreateParams({
         poolPercent: 100e2,
@@ -176,11 +180,24 @@ abstract contract UniswapV2DirectToLiquidityTest is Test, Permit2User, WithSalts
         _;
     }
 
+    modifier givenQuoteTokenDecimals(uint8 decimals_) {
+        _quoteToken = new MockERC20("Quote Token", "QT", decimals_);
+        _;
+    }
+
+    modifier givenBaseTokenDecimals(uint8 decimals_) {
+        _baseToken = new MockERC20("Base Token", "BT", decimals_);
+
+        // Scale the capacity
+        _lotCapacity = uint96(_LOT_CAPACITY * 10 ** decimals_ / 10 ** 18);
+        _;
+    }
+
     function _createLot(address seller_, bytes memory err_) internal returns (uint96 lotId) {
         // Mint and approve the capacity to the owner
-        _baseToken.mint(seller_, _LOT_CAPACITY);
+        _baseToken.mint(seller_, _lotCapacity);
         vm.prank(seller_);
-        _baseToken.approve(address(_auctionHouse), _LOT_CAPACITY);
+        _baseToken.approve(address(_auctionHouse), _lotCapacity);
 
         // Prep the lot arguments
         IAuctionHouse.RoutingParams memory routingParams = IAuctionHouse.RoutingParams({
@@ -200,7 +217,7 @@ abstract contract UniswapV2DirectToLiquidityTest is Test, Permit2User, WithSalts
             start: _AUCTION_START,
             duration: _DURATION,
             capacityInQuote: false,
-            capacity: _LOT_CAPACITY,
+            capacity: _lotCapacity,
             implParams: abi.encode("")
         });
 
@@ -229,7 +246,7 @@ abstract contract UniswapV2DirectToLiquidityTest is Test, Permit2User, WithSalts
             seller_,
             address(_baseToken),
             address(_quoteToken),
-            _LOT_CAPACITY,
+            _lotCapacity,
             false,
             abi.encode(_dtlCreateParams)
         );