Spearbit 89 Unsafe casting and overflow of negation

Uniswap · Sep 5, 2024 · ae71fc6 · ae71fc6
1 parent a0ef70d
commit ae71fc6
Show file tree

Hide file tree

Showing 49 changed files with 58 additions and 55 deletions.
diff --git a/.forge-snapshots/Payments_swap_settleFromCaller_takeAllToMsgSender.snap b/.forge-snapshots/Payments_swap_settleFromCaller_takeAllToMsgSender.snap
@@ -1 +1 @@
-129459
+129465
diff --git a/.forge-snapshots/Payments_swap_settleFromCaller_takeAllToSpecifiedAddress.snap b/.forge-snapshots/Payments_swap_settleFromCaller_takeAllToSpecifiedAddress.snap
@@ -1 +1 @@
-131377
+131383
diff --git a/.forge-snapshots/Payments_swap_settleWithBalance_takeAllToMsgSender.snap b/.forge-snapshots/Payments_swap_settleWithBalance_takeAllToMsgSender.snap
@@ -1 +1 @@
-123612
+123588
diff --git a/.forge-snapshots/Payments_swap_settleWithBalance_takeAllToSpecifiedAddress.snap b/.forge-snapshots/Payments_swap_settleWithBalance_takeAllToSpecifiedAddress.snap
@@ -1 +1 @@
-123754
+123730
diff --git a/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withClose.snap b/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withClose.snap
@@ -1 +1 @@
-154735
+154692
diff --git a/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withSettlePair.snap b/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withSettlePair.snap
@@ -1 +1 @@
-153793
+153750
diff --git a/.forge-snapshots/PositionManager_increaseLiquidity_native.snap b/.forge-snapshots/PositionManager_increaseLiquidity_native.snap
@@ -1 +1 @@
-136186
+136143
diff --git a/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap b/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap
@@ -1 +1 @@
-132042
+131999
diff --git a/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap b/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap
@@ -1 +1 @@
-172746
+172703
diff --git a/.forge-snapshots/PositionManager_increase_autocompound_clearExcess.snap b/.forge-snapshots/PositionManager_increase_autocompound_clearExcess.snap
@@ -1 +1 @@
-143428
+143385
diff --git a/.forge-snapshots/PositionManager_mint_native.snap b/.forge-snapshots/PositionManager_mint_native.snap
@@ -1 +1 @@
-339884
+339841
diff --git a/.forge-snapshots/PositionManager_mint_nativeWithSweep_withClose.snap b/.forge-snapshots/PositionManager_mint_nativeWithSweep_withClose.snap
@@ -1 +1 @@
-348264
+348221
diff --git a/.forge-snapshots/PositionManager_mint_nativeWithSweep_withSettlePair.snap b/.forge-snapshots/PositionManager_mint_nativeWithSweep_withSettlePair.snap
@@ -1 +1 @@
-347630
+347587
diff --git a/.forge-snapshots/PositionManager_mint_onSameTickLower.snap b/.forge-snapshots/PositionManager_mint_onSameTickLower.snap
@@ -1 +1 @@
-318202
+318159
diff --git a/.forge-snapshots/PositionManager_mint_onSameTickUpper.snap b/.forge-snapshots/PositionManager_mint_onSameTickUpper.snap
@@ -1 +1 @@
-318872
+318829
diff --git a/.forge-snapshots/PositionManager_mint_sameRange.snap b/.forge-snapshots/PositionManager_mint_sameRange.snap
@@ -1 +1 @@
-244441
+244398
diff --git a/.forge-snapshots/PositionManager_mint_settleWithBalance_sweep.snap b/.forge-snapshots/PositionManager_mint_settleWithBalance_sweep.snap
@@ -1 +1 @@
-374127
+374084
diff --git a/.forge-snapshots/PositionManager_mint_warmedPool_differentRange.snap b/.forge-snapshots/PositionManager_mint_warmedPool_differentRange.snap
@@ -1 +1 @@
-324233
+324190
diff --git a/.forge-snapshots/PositionManager_mint_withClose.snap b/.forge-snapshots/PositionManager_mint_withClose.snap
@@ -1 +1 @@
-375533
+375490
diff --git a/.forge-snapshots/PositionManager_mint_withSettlePair.snap b/.forge-snapshots/PositionManager_mint_withSettlePair.snap
@@ -1 +1 @@
-374733
+374690
diff --git a/.forge-snapshots/PositionManager_multicall_initialize_mint.snap b/.forge-snapshots/PositionManager_multicall_initialize_mint.snap
@@ -1 +1 @@
-419680
+419637
diff --git a/.forge-snapshots/V4Router_Bytecode.snap b/.forge-snapshots/V4Router_Bytecode.snap
@@ -1 +1 @@
-6909
+6865
diff --git a/.forge-snapshots/V4Router_ExactIn1Hop_nativeIn.snap b/.forge-snapshots/V4Router_ExactIn1Hop_nativeIn.snap
@@ -1 +1 @@
-115155
+115185
diff --git a/.forge-snapshots/V4Router_ExactIn1Hop_nativeOut.snap b/.forge-snapshots/V4Router_ExactIn1Hop_nativeOut.snap
@@ -1 +1 @@
-115632
+115662
diff --git a/.forge-snapshots/V4Router_ExactIn1Hop_oneForZero.snap b/.forge-snapshots/V4Router_ExactIn1Hop_oneForZero.snap
@@ -1 +1 @@
-124447
+124477
diff --git a/.forge-snapshots/V4Router_ExactIn1Hop_zeroForOne.snap b/.forge-snapshots/V4Router_ExactIn1Hop_zeroForOne.snap
@@ -1 +1 @@
-130165
+130195
diff --git a/.forge-snapshots/V4Router_ExactIn2Hops.snap b/.forge-snapshots/V4Router_ExactIn2Hops.snap
@@ -1 +1 @@
-179173
+179203
diff --git a/.forge-snapshots/V4Router_ExactIn2Hops_nativeIn.snap b/.forge-snapshots/V4Router_ExactIn2Hops_nativeIn.snap
@@ -1 +1 @@
-169873
+169903
diff --git a/.forge-snapshots/V4Router_ExactIn3Hops.snap b/.forge-snapshots/V4Router_ExactIn3Hops.snap
@@ -1 +1 @@
-228160
+228190
diff --git a/.forge-snapshots/V4Router_ExactIn3Hops_nativeIn.snap b/.forge-snapshots/V4Router_ExactIn3Hops_nativeIn.snap
@@ -1 +1 @@
-218884
+218914
diff --git a/.forge-snapshots/V4Router_ExactInputSingle.snap b/.forge-snapshots/V4Router_ExactInputSingle.snap
@@ -1 +1 @@
-129459
+129465
diff --git a/.forge-snapshots/V4Router_ExactInputSingle_nativeIn.snap b/.forge-snapshots/V4Router_ExactInputSingle_nativeIn.snap
@@ -1 +1 @@
-114449
+114455
diff --git a/.forge-snapshots/V4Router_ExactInputSingle_nativeOut.snap b/.forge-snapshots/V4Router_ExactInputSingle_nativeOut.snap
@@ -1 +1 @@
-114895
+114901
diff --git a/.forge-snapshots/V4Router_ExactOut1Hop_nativeIn_sweepETH.snap b/.forge-snapshots/V4Router_ExactOut1Hop_nativeIn_sweepETH.snap
@@ -1 +1 @@
-121352
+121378
diff --git a/.forge-snapshots/V4Router_ExactOut1Hop_nativeOut.snap b/.forge-snapshots/V4Router_ExactOut1Hop_nativeOut.snap
@@ -1 +1 @@
-116653
+116679
diff --git a/.forge-snapshots/V4Router_ExactOut1Hop_oneForZero.snap b/.forge-snapshots/V4Router_ExactOut1Hop_oneForZero.snap
@@ -1 +1 @@
-125468
+125494
diff --git a/.forge-snapshots/V4Router_ExactOut1Hop_zeroForOne.snap b/.forge-snapshots/V4Router_ExactOut1Hop_zeroForOne.snap
@@ -1 +1 @@
-129385
+129411
diff --git a/.forge-snapshots/V4Router_ExactOut2Hops.snap b/.forge-snapshots/V4Router_ExactOut2Hops.snap
@@ -1 +1 @@
-179217
+179239
diff --git a/.forge-snapshots/V4Router_ExactOut2Hops_nativeIn.snap b/.forge-snapshots/V4Router_ExactOut2Hops_nativeIn.snap
@@ -1 +1 @@
-175101
+175123
diff --git a/.forge-snapshots/V4Router_ExactOut3Hops.snap b/.forge-snapshots/V4Router_ExactOut3Hops.snap
@@ -1 +1 @@
-229056
+229074
diff --git a/.forge-snapshots/V4Router_ExactOut3Hops_nativeIn.snap b/.forge-snapshots/V4Router_ExactOut3Hops_nativeIn.snap
@@ -1 +1 @@
-224964
+224982
diff --git a/.forge-snapshots/V4Router_ExactOut3Hops_nativeOut.snap b/.forge-snapshots/V4Router_ExactOut3Hops_nativeOut.snap
@@ -1 +1 @@
-220265
+220283
diff --git a/.forge-snapshots/V4Router_ExactOutputSingle.snap b/.forge-snapshots/V4Router_ExactOutputSingle.snap
@@ -1 +1 @@
-128666
+128684
diff --git a/.forge-snapshots/V4Router_ExactOutputSingle_nativeIn_sweepETH.snap b/.forge-snapshots/V4Router_ExactOutputSingle_nativeIn_sweepETH.snap
@@ -1 +1 @@
-120633
+120651
diff --git a/.forge-snapshots/V4Router_ExactOutputSingle_nativeOut.snap b/.forge-snapshots/V4Router_ExactOutputSingle_nativeOut.snap
@@ -1 +1 @@
-116009
+116027
diff --git a/src/PositionManager.sol b/src/PositionManager.sol
@@ -365,6 +365,7 @@ contract PositionManager is
         // the locker is the payer or receiver
         address caller = msgSender();
         if (currencyDelta < 0) {
+            // Casting is safe due to limits on the total supply of a pool
             _settle(currency, caller, uint256(-currencyDelta));
         } else if (currencyDelta > 0) {
             _take(currency, caller, uint256(currencyDelta));
@@ -417,6 +418,7 @@ contract PositionManager is
         if (payer == address(this)) {
             currency.transfer(address(poolManager), amount);
         } else {
+            // Casting from uint256 to uint160 is safe due to limits on the total supply of a pool
             permit2.transferFrom(payer, address(poolManager), uint160(amount), Currency.unwrap(currency));
         }
     }

diff --git a/src/V4Router.sol b/src/V4Router.sol
@@ -91,7 +91,7 @@ abstract contract V4Router is IV4Router, BaseActionsRouter, DeltaResolver {
                 _getFullCredit(params.zeroForOne ? params.poolKey.currency0 : params.poolKey.currency1).toUint128();
         }
         uint128 amountOut = _swap(
-            params.poolKey, params.zeroForOne, int256(-int128(amountIn)), params.sqrtPriceLimitX96, params.hookData
+            params.poolKey, params.zeroForOne, -int256(uint256(amountIn)), params.sqrtPriceLimitX96, params.hookData
         ).toUint128();
         if (amountOut < params.amountOutMinimum) revert V4TooLittleReceived(params.amountOutMinimum, amountOut);
     }
@@ -122,14 +122,14 @@ abstract contract V4Router is IV4Router, BaseActionsRouter, DeltaResolver {
 
     function _swapExactOutputSingle(IV4Router.ExactOutputSingleParams calldata params) private {
         uint128 amountIn = (
-            -_swap(
+            uint256(-int256(_swap(
                 params.poolKey,
                 params.zeroForOne,
-                int256(int128(params.amountOut)),
+                int256(uint256(params.amountOut)),
                 params.sqrtPriceLimitX96,
                 params.hookData
             )
-        ).toUint128();
+        ))).toUint128();
         if (amountIn > params.amountInMaximum) revert V4TooMuchRequested(params.amountInMaximum, amountIn);
     }
 
@@ -146,7 +146,7 @@ abstract contract V4Router is IV4Router, BaseActionsRouter, DeltaResolver {
                 pathKey = params.path[i - 1];
                 (PoolKey memory poolKey, bool oneForZero) = pathKey.getPoolAndSwapDirection(currencyOut);
                 // The output delta will always be negative, except for when interacting with certain hook pools
-                amountIn = (-_swap(poolKey, !oneForZero, int256(uint256(amountOut)), 0, pathKey.hookData)).toUint128();
+                amountIn = (uint256(-int256(_swap(poolKey, !oneForZero, int256(uint256(amountOut)), 0, pathKey.hookData)))).toUint128();
 
                 amountOut = amountIn;
                 currencyOut = pathKey.intermediateCurrency;

diff --git a/src/base/DeltaResolver.sol b/src/base/DeltaResolver.sol
@@ -54,6 +54,7 @@ abstract contract DeltaResolver is ImmutableState {
         int256 _amount = poolManager.currencyDelta(address(this), currency);
         // If the amount is positive, it should be taken not settled.
         if (_amount > 0) revert DeltaNotNegative(currency);
+        // Casting is safe due to limits on the total supply of a pool
         amount = uint256(-_amount);
     }
 

diff --git a/src/libraries/SlippageCheck.sol b/src/libraries/SlippageCheck.sol
@@ -9,7 +9,7 @@ import {SafeCast} from "@uniswap/v4-core/src/libraries/SafeCast.sol";
 library SlippageCheck {
     using SafeCast for int128;
 
-    error MaximumAmountExceeded(uint128 maximumAmount, uint128 amountRequested);
+    error MaximumAmountExceeded(uint256 maximumAmount, uint256 amountRequested);
     error MinimumAmountInsufficient(uint128 minimumAmount, uint128 amountReceived);
 
     /// @notice Revert if one or both deltas does not meet a minimum output
@@ -41,9 +41,9 @@ library SlippageCheck {
         // Thus, we only cast the delta if it is guaranteed to be negative.
         // And we do NOT revert in the positive delta case. Since a positive delta means the hook is crediting tokens to the user for minting/increasing liquidity, we do not check slippage.
         // This means this contract will NOT support _positive_ slippage checks (minAmountOut checks) on pools where the hook returns a positive delta on mint/increase.
-        int128 amount0 = delta.amount0();
-        int128 amount1 = delta.amount1();
-        if (amount0 < 0 && amount0Max < uint128(-amount0)) revert MaximumAmountExceeded(amount0Max, uint128(-amount0));
-        if (amount1 < 0 && amount1Max < uint128(-amount1)) revert MaximumAmountExceeded(amount1Max, uint128(-amount1));
+        int256 amount0 = delta.amount0();
+        int256 amount1 = delta.amount1();
+        if (amount0 < 0 && amount0Max < uint256(-amount0)) revert MaximumAmountExceeded(uint256(amount0Max), uint256(-amount0));
+        if (amount1 < 0 && amount1Max < uint256(-amount1)) revert MaximumAmountExceeded(uint256(amount1Max), uint256(-amount1));
     }
 }