Fix length check

Uniswap · Sep 6, 2024 · c6397f0 · c6397f0
1 parent e1501a8
commit c6397f0
Show file tree

Hide file tree

Showing 69 changed files with 84 additions and 77 deletions.
diff --git a/.forge-snapshots/BaseActionsRouter_mock10commands.snap b/.forge-snapshots/BaseActionsRouter_mock10commands.snap
@@ -1 +1 @@
-60529
+60677
diff --git a/.forge-snapshots/Payments_swap_settleFromCaller_takeAllToMsgSender.snap b/.forge-snapshots/Payments_swap_settleFromCaller_takeAllToMsgSender.snap
@@ -1 +1 @@
-129802
+129854
diff --git a/.forge-snapshots/Payments_swap_settleFromCaller_takeAllToSpecifiedAddress.snap b/.forge-snapshots/Payments_swap_settleFromCaller_takeAllToSpecifiedAddress.snap
@@ -1 +1 @@
-131842
+131905
diff --git a/.forge-snapshots/Payments_swap_settleWithBalance_takeAllToMsgSender.snap b/.forge-snapshots/Payments_swap_settleWithBalance_takeAllToMsgSender.snap
@@ -1 +1 @@
-124047
+124110
diff --git a/.forge-snapshots/Payments_swap_settleWithBalance_takeAllToSpecifiedAddress.snap b/.forge-snapshots/Payments_swap_settleWithBalance_takeAllToSpecifiedAddress.snap
@@ -1 +1 @@
-124189
+124252
diff --git a/.forge-snapshots/PositionManager_burn_empty.snap b/.forge-snapshots/PositionManager_burn_empty.snap
@@ -1 +1 @@
-50396
+50440
diff --git a/.forge-snapshots/PositionManager_burn_empty_native.snap b/.forge-snapshots/PositionManager_burn_empty_native.snap
@@ -1 +1 @@
-50396
+50440
diff --git a/.forge-snapshots/PositionManager_burn_nonEmpty_native_withClose.snap b/.forge-snapshots/PositionManager_burn_nonEmpty_native_withClose.snap
@@ -1 +1 @@
-125497
+125578
diff --git a/.forge-snapshots/PositionManager_burn_nonEmpty_native_withTakePair.snap b/.forge-snapshots/PositionManager_burn_nonEmpty_native_withTakePair.snap
@@ -1 +1 @@
-124960
+125025
diff --git a/.forge-snapshots/PositionManager_burn_nonEmpty_withClose.snap b/.forge-snapshots/PositionManager_burn_nonEmpty_withClose.snap
@@ -1 +1 @@
-132348
+132431
diff --git a/.forge-snapshots/PositionManager_burn_nonEmpty_withTakePair.snap b/.forge-snapshots/PositionManager_burn_nonEmpty_withTakePair.snap
@@ -1 +1 @@
-131811
+131878
diff --git a/.forge-snapshots/PositionManager_collect_native.snap b/.forge-snapshots/PositionManager_collect_native.snap
@@ -1 +1 @@
-146180
+146281
diff --git a/.forge-snapshots/PositionManager_collect_sameRange.snap b/.forge-snapshots/PositionManager_collect_sameRange.snap
@@ -1 +1 @@
-154743
+154847
diff --git a/.forge-snapshots/PositionManager_collect_withClose.snap b/.forge-snapshots/PositionManager_collect_withClose.snap
@@ -1 +1 @@
-154743
+154847
diff --git a/.forge-snapshots/PositionManager_collect_withTakePair.snap b/.forge-snapshots/PositionManager_collect_withTakePair.snap
@@ -1 +1 @@
-154084
+154168
diff --git a/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap b/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap
@@ -1 +1 @@
-111889
+111970
diff --git a/.forge-snapshots/PositionManager_decreaseLiquidity_withClose.snap b/.forge-snapshots/PositionManager_decreaseLiquidity_withClose.snap
@@ -1 +1 @@
-119624
+119728
diff --git a/.forge-snapshots/PositionManager_decreaseLiquidity_withTakePair.snap b/.forge-snapshots/PositionManager_decreaseLiquidity_withTakePair.snap
@@ -1 +1 @@
-118965
+119049
diff --git a/.forge-snapshots/PositionManager_decrease_burnEmpty.snap b/.forge-snapshots/PositionManager_decrease_burnEmpty.snap
@@ -1 +1 @@
-135151
+135250
diff --git a/.forge-snapshots/PositionManager_decrease_burnEmpty_native.snap b/.forge-snapshots/PositionManager_decrease_burnEmpty_native.snap
@@ -1 +1 @@
-128300
+128397
diff --git a/.forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap b/.forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap
@@ -1 +1 @@
-132311
+132415
diff --git a/.forge-snapshots/PositionManager_decrease_take_take.snap b/.forge-snapshots/PositionManager_decrease_take_take.snap
@@ -1 +1 @@
-120200
+120304
diff --git a/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withClose.snap b/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withClose.snap
@@ -1 +1 @@
-159205
+159032
diff --git a/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withSettlePair.snap b/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withSettlePair.snap
@@ -1 +1 @@
-158165
+157972
diff --git a/.forge-snapshots/PositionManager_increaseLiquidity_native.snap b/.forge-snapshots/PositionManager_increaseLiquidity_native.snap
@@ -1 +1 @@
-140884
+140859
diff --git a/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap b/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap
@@ -1 +1 @@
-136268
+136346
diff --git a/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap b/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap
@@ -1 +1 @@
-177216
+177339
diff --git a/.forge-snapshots/PositionManager_increase_autocompound_clearExcess.snap b/.forge-snapshots/PositionManager_increase_autocompound_clearExcess.snap
@@ -1 +1 @@
-147898
+148015
diff --git a/.forge-snapshots/PositionManager_mint_native.snap b/.forge-snapshots/PositionManager_mint_native.snap
@@ -1 +1 @@
-364754
+364729
diff --git a/.forge-snapshots/PositionManager_mint_nativeWithSweep_withClose.snap b/.forge-snapshots/PositionManager_mint_nativeWithSweep_withClose.snap
@@ -1 +1 @@
-373256
+373252
diff --git a/.forge-snapshots/PositionManager_mint_nativeWithSweep_withSettlePair.snap b/.forge-snapshots/PositionManager_mint_nativeWithSweep_withSettlePair.snap
@@ -1 +1 @@
-372500
+372475
diff --git a/.forge-snapshots/PositionManager_mint_onSameTickLower.snap b/.forge-snapshots/PositionManager_mint_onSameTickLower.snap
@@ -1 +1 @@
-317750
+317577
diff --git a/.forge-snapshots/PositionManager_mint_onSameTickUpper.snap b/.forge-snapshots/PositionManager_mint_onSameTickUpper.snap
@@ -1 +1 @@
-318420
+318247
diff --git a/.forge-snapshots/PositionManager_mint_sameRange.snap b/.forge-snapshots/PositionManager_mint_sameRange.snap
@@ -1 +1 @@
-243989
+243816
diff --git a/.forge-snapshots/PositionManager_mint_settleWithBalance_sweep.snap b/.forge-snapshots/PositionManager_mint_settleWithBalance_sweep.snap
@@ -1 +1 @@
-419141
+419008
diff --git a/.forge-snapshots/PositionManager_mint_warmedPool_differentRange.snap b/.forge-snapshots/PositionManager_mint_warmedPool_differentRange.snap
@@ -1 +1 @@
-323781
+323608
diff --git a/.forge-snapshots/PositionManager_mint_withClose.snap b/.forge-snapshots/PositionManager_mint_withClose.snap
@@ -1 +1 @@
-420303
+420130
diff --git a/.forge-snapshots/PositionManager_mint_withSettlePair.snap b/.forge-snapshots/PositionManager_mint_withSettlePair.snap
@@ -1 +1 @@
-419381
+419188
diff --git a/.forge-snapshots/PositionManager_multicall_initialize_mint.snap b/.forge-snapshots/PositionManager_multicall_initialize_mint.snap
@@ -1 +1 @@
-464471
+464302
diff --git a/.forge-snapshots/StateView_extsload_getFeeGrowthGlobals.snap b/.forge-snapshots/StateView_extsload_getFeeGrowthGlobals.snap
@@ -1 +1 @@
-2256
+2259
diff --git a/.forge-snapshots/StateView_extsload_getFeeGrowthInside.snap b/.forge-snapshots/StateView_extsload_getFeeGrowthInside.snap
@@ -1 +1 @@
-7994
+8003
diff --git a/.forge-snapshots/StateView_extsload_getPositionInfo.snap b/.forge-snapshots/StateView_extsload_getPositionInfo.snap
@@ -1 +1 @@
-2826
+2829
diff --git a/.forge-snapshots/StateView_extsload_getTickFeeGrowthOutside.snap b/.forge-snapshots/StateView_extsload_getTickFeeGrowthOutside.snap
@@ -1 +1 @@
-2543
+2546
diff --git a/.forge-snapshots/StateView_extsload_getTickInfo.snap b/.forge-snapshots/StateView_extsload_getTickInfo.snap
@@ -1 +1 @@
-2758
+2761
diff --git a/.forge-snapshots/V4Router_Bytecode.snap b/.forge-snapshots/V4Router_Bytecode.snap
@@ -1 +1 @@
-7133
+7148
diff --git a/.forge-snapshots/V4Router_ExactIn1Hop_nativeIn.snap b/.forge-snapshots/V4Router_ExactIn1Hop_nativeIn.snap
@@ -1 +1 @@
-115522
+115722
diff --git a/.forge-snapshots/V4Router_ExactIn1Hop_nativeOut.snap b/.forge-snapshots/V4Router_ExactIn1Hop_nativeOut.snap
@@ -1 +1 @@
-115999
+116043
diff --git a/.forge-snapshots/V4Router_ExactIn1Hop_oneForZero.snap b/.forge-snapshots/V4Router_ExactIn1Hop_oneForZero.snap
@@ -1 +1 @@
-124814
+124861
diff --git a/.forge-snapshots/V4Router_ExactIn1Hop_zeroForOne.snap b/.forge-snapshots/V4Router_ExactIn1Hop_zeroForOne.snap
@@ -1 +1 @@
-130532
+130584
diff --git a/.forge-snapshots/V4Router_ExactIn2Hops.snap b/.forge-snapshots/V4Router_ExactIn2Hops.snap
@@ -1 +1 @@
-179540
+179724
diff --git a/.forge-snapshots/V4Router_ExactIn2Hops_nativeIn.snap b/.forge-snapshots/V4Router_ExactIn2Hops_nativeIn.snap
@@ -1 +1 @@
-170240
+170577
diff --git a/.forge-snapshots/V4Router_ExactIn3Hops.snap b/.forge-snapshots/V4Router_ExactIn3Hops.snap
@@ -1 +1 @@
-228527
+228843
diff --git a/.forge-snapshots/V4Router_ExactIn3Hops_nativeIn.snap b/.forge-snapshots/V4Router_ExactIn3Hops_nativeIn.snap
@@ -1 +1 @@
-219251
+219720
diff --git a/.forge-snapshots/V4Router_ExactInputSingle.snap b/.forge-snapshots/V4Router_ExactInputSingle.snap
@@ -1 +1 @@
-129802
+129854
diff --git a/.forge-snapshots/V4Router_ExactInputSingle_nativeIn.snap b/.forge-snapshots/V4Router_ExactInputSingle_nativeIn.snap
@@ -1 +1 @@
-114792
+114992
diff --git a/.forge-snapshots/V4Router_ExactInputSingle_nativeOut.snap b/.forge-snapshots/V4Router_ExactInputSingle_nativeOut.snap
@@ -1 +1 @@
-115238
+115282
diff --git a/.forge-snapshots/V4Router_ExactOut1Hop_nativeIn_sweepETH.snap b/.forge-snapshots/V4Router_ExactOut1Hop_nativeIn_sweepETH.snap
@@ -1 +1 @@
-121750
+121985
diff --git a/.forge-snapshots/V4Router_ExactOut1Hop_nativeOut.snap b/.forge-snapshots/V4Router_ExactOut1Hop_nativeOut.snap
@@ -1 +1 @@
-117051
+117107
diff --git a/.forge-snapshots/V4Router_ExactOut1Hop_oneForZero.snap b/.forge-snapshots/V4Router_ExactOut1Hop_oneForZero.snap
@@ -1 +1 @@
-125866
+125925
diff --git a/.forge-snapshots/V4Router_ExactOut1Hop_zeroForOne.snap b/.forge-snapshots/V4Router_ExactOut1Hop_zeroForOne.snap
@@ -1 +1 @@
-129783
+129870
diff --git a/.forge-snapshots/V4Router_ExactOut2Hops.snap b/.forge-snapshots/V4Router_ExactOut2Hops.snap
@@ -1 +1 @@
-179611
+179842
diff --git a/.forge-snapshots/V4Router_ExactOut2Hops_nativeIn.snap b/.forge-snapshots/V4Router_ExactOut2Hops_nativeIn.snap
@@ -1 +1 @@
-175495
+175902
diff --git a/.forge-snapshots/V4Router_ExactOut3Hops.snap b/.forge-snapshots/V4Router_ExactOut3Hops.snap
@@ -1 +1 @@
-229446
+229821
diff --git a/.forge-snapshots/V4Router_ExactOut3Hops_nativeIn.snap b/.forge-snapshots/V4Router_ExactOut3Hops_nativeIn.snap
@@ -1 +1 @@
-225354
+225905
diff --git a/.forge-snapshots/V4Router_ExactOut3Hops_nativeOut.snap b/.forge-snapshots/V4Router_ExactOut3Hops_nativeOut.snap
@@ -1 +1 @@
-220655
+221027
diff --git a/.forge-snapshots/V4Router_ExactOutputSingle.snap b/.forge-snapshots/V4Router_ExactOutputSingle.snap
@@ -1 +1 @@
-129053
+129140
diff --git a/.forge-snapshots/V4Router_ExactOutputSingle_nativeIn_sweepETH.snap b/.forge-snapshots/V4Router_ExactOutputSingle_nativeIn_sweepETH.snap
@@ -1 +1 @@
-121020
+121255
diff --git a/.forge-snapshots/V4Router_ExactOutputSingle_nativeOut.snap b/.forge-snapshots/V4Router_ExactOutputSingle_nativeOut.snap
@@ -1 +1 @@
-116396
+116452
diff --git a/src/libraries/CalldataDecoder.sol b/src/libraries/CalldataDecoder.sol
@@ -16,7 +16,7 @@ library CalldataDecoder {
     ///      (note that this does deviate from standard solidity behavior and offsets/lengths will
     ///      be interpreted as mod type(uint32).max which will only impact malicious/buggy callers)
     uint256 constant OFFSET_OR_LENGTH_MASK = 0xffffffff;
-    uint256 constant OFFSET_OR_LENGTH_MASK_WITH_PADDING = 0xffffffe0;
+    uint256 constant OFFSET_OR_LENGTH_MASK_AND_WORD_ALIGN = 0xffffffe0;
 
     /// @notice equivalent to SliceOutOfBounds.selector, stored in least-significant bits
     uint256 constant SLICE_ERROR_SELECTOR = 0x3b99b53d;
@@ -32,31 +32,38 @@ library CalldataDecoder {
             // 0x00: 0x40 (offset to `actions.length`)
             // 0x20: 0x60 + actions.length (offset to `params.length`)
             // 0x40: `actions.length`
+            // 0x60: beginning of actions
+
+            // Verify actions offset matches strict encoding
             let invalidData := xor(calldataload(_bytes.offset), 0x40)
             actions.offset := add(_bytes.offset, 0x60)
             actions.length := and(calldataload(add(_bytes.offset, 0x40)), OFFSET_OR_LENGTH_MASK)
 
-            let paramsLengthOffset := add(and(add(actions.length, 0x1f), OFFSET_OR_LENGTH_MASK_WITH_PADDING), 0x60)
-            // Verify actions offset matches strict encoding
+            // Round actions length up to be word-aligned, and add 0x60 (for the first 3 words of encoding)
+            let paramsLengthOffset := add(and(add(actions.length, 0x1f), OFFSET_OR_LENGTH_MASK_AND_WORD_ALIGN), 0x60)
+            // Verify params offset matches strict encoding
             invalidData := or(invalidData, xor(calldataload(add(_bytes.offset, 0x20)), paramsLengthOffset))
             let paramsLengthPointer := add(_bytes.offset, paramsLengthOffset)
             params.length := and(calldataload(paramsLengthPointer), OFFSET_OR_LENGTH_MASK)
             params.offset := add(paramsLengthPointer, 0x20)
 
-            // Expected head offset for `params[0]` is params.length * 32
+            // Expected offset for `params[0]` is params.length * 32
+            // As the first `params.length` slots are pointers to each of the array element lengths
             let tailOffset := shl(5, params.length)
             let expectedOffset := tailOffset
 
             for { let offset := 0 } lt(offset, tailOffset) { offset := add(offset, 32) } {
-                let cdOffsetItemLength := calldataload(add(params.offset, offset))
-                invalidData := or(invalidData, xor(cdOffsetItemLength, expectedOffset))
-                let cdPtrItemLength := add(params.offset, cdOffsetItemLength)
+                let itemLengthOffset := calldataload(add(params.offset, offset))
+                // Verify that the offset matches the expected offset from strict encoding
+                invalidData := or(invalidData, xor(itemLengthOffset, expectedOffset))
+                let itemLengthPointer := add(params.offset, itemLengthOffset)
                 let length :=
-                    add(and(add(calldataload(cdPtrItemLength), 0x1f), OFFSET_OR_LENGTH_MASK_WITH_PADDING), 0x20)
+                    add(and(add(calldataload(itemLengthPointer), 0x1f), OFFSET_OR_LENGTH_MASK_AND_WORD_ALIGN), 0x20)
                 expectedOffset := add(expectedOffset, length)
             }
 
-            if invalidData {
+            // if the data encoding was invalid, or the provided bytes string isnt as long as the encoding says, revert
+            if or(invalidData, lt(add(_bytes.length, _bytes.offset), add(params.offset, expectedOffset))) {
                 mstore(0, SLICE_ERROR_SELECTOR)
                 revert(0x1c, 4)
             }