From 151b28293733b9cd8a310babc15408fe1ba55c35 Mon Sep 17 00:00:00 2001 From: Alice <34962750+hensha256@users.noreply.github.com> Date: Thu, 5 Sep 2024 20:42:41 -0400 Subject: [PATCH] OZ L02; Spearbit 87; Enforce strict abi encoding (#346) * improved calldata checks * remove console * remove toLengthOffset function * natspec correction * mask values to be within 32 bits * verify array elements are encoded in order * mask straight after calldata load * masking in toBytes, remove selector constant * fuzz test * add testing * fix for 0 array length * put alt. calldata decoder back (#344) Co-authored-by: 0age <0age@protonmail.org> * core on main for debug profile * Fix length check * optimise toBytes * linting --------- Co-authored-by: 0age <37939117+0age@users.noreply.github.com> Co-authored-by: 0age <0age@protonmail.org> --- .../BaseActionsRouter_mock10commands.snap | 2 +- ...p_settleFromCaller_takeAllToMsgSender.snap | 2 +- ...eFromCaller_takeAllToSpecifiedAddress.snap | 2 +- ..._settleWithBalance_takeAllToMsgSender.snap | 2 +- ...WithBalance_takeAllToSpecifiedAddress.snap | 2 +- .../PositionManager_burn_empty.snap | 2 +- .../PositionManager_burn_empty_native.snap | 2 +- ...anager_burn_nonEmpty_native_withClose.snap | 2 +- ...ger_burn_nonEmpty_native_withTakePair.snap | 2 +- ...sitionManager_burn_nonEmpty_withClose.snap | 2 +- ...ionManager_burn_nonEmpty_withTakePair.snap | 2 +- .../PositionManager_collect_native.snap | 2 +- .../PositionManager_collect_sameRange.snap | 2 +- .../PositionManager_collect_withClose.snap | 2 +- .../PositionManager_collect_withTakePair.snap | 2 +- ...itionManager_decreaseLiquidity_native.snap | 2 +- ...onManager_decreaseLiquidity_withClose.snap | 2 +- ...anager_decreaseLiquidity_withTakePair.snap | 2 +- .../PositionManager_decrease_burnEmpty.snap | 2 +- ...tionManager_decrease_burnEmpty_native.snap | 2 +- ...nager_decrease_sameRange_allLiquidity.snap | 2 +- .../PositionManager_decrease_take_take.snap | 2 +- ...ger_increaseLiquidity_erc20_withClose.snap | 2 +- ...ncreaseLiquidity_erc20_withSettlePair.snap | 2 +- ...itionManager_increaseLiquidity_native.snap | 2 +- ...crease_autocompoundExactUnclaimedFees.snap | 2 +- ...increase_autocompoundExcessFeesCredit.snap | 2 +- ...ger_increase_autocompound_clearExcess.snap | 2 +- .../PositionManager_mint_native.snap | 2 +- ...anager_mint_nativeWithSweep_withClose.snap | 2 +- ...r_mint_nativeWithSweep_withSettlePair.snap | 2 +- .../PositionManager_mint_onSameTickLower.snap | 2 +- .../PositionManager_mint_onSameTickUpper.snap | 2 +- .../PositionManager_mint_sameRange.snap | 2 +- ...nManager_mint_settleWithBalance_sweep.snap | 2 +- ...anager_mint_warmedPool_differentRange.snap | 2 +- .../PositionManager_mint_withClose.snap | 2 +- .../PositionManager_mint_withSettlePair.snap | 2 +- ...tionManager_multicall_initialize_mint.snap | 2 +- .forge-snapshots/V4Router_Bytecode.snap | 2 +- .../V4Router_ExactIn1Hop_nativeIn.snap | 2 +- .../V4Router_ExactIn1Hop_nativeOut.snap | 2 +- .../V4Router_ExactIn1Hop_oneForZero.snap | 2 +- .../V4Router_ExactIn1Hop_zeroForOne.snap | 2 +- .forge-snapshots/V4Router_ExactIn2Hops.snap | 2 +- .../V4Router_ExactIn2Hops_nativeIn.snap | 2 +- .forge-snapshots/V4Router_ExactIn3Hops.snap | 2 +- .../V4Router_ExactIn3Hops_nativeIn.snap | 2 +- .../V4Router_ExactInputSingle.snap | 2 +- .../V4Router_ExactInputSingle_nativeIn.snap | 2 +- .../V4Router_ExactInputSingle_nativeOut.snap | 2 +- ...Router_ExactOut1Hop_nativeIn_sweepETH.snap | 2 +- .../V4Router_ExactOut1Hop_nativeOut.snap | 2 +- .../V4Router_ExactOut1Hop_oneForZero.snap | 2 +- .../V4Router_ExactOut1Hop_zeroForOne.snap | 2 +- .forge-snapshots/V4Router_ExactOut2Hops.snap | 2 +- .../V4Router_ExactOut2Hops_nativeIn.snap | 2 +- .forge-snapshots/V4Router_ExactOut3Hops.snap | 2 +- .../V4Router_ExactOut3Hops_nativeIn.snap | 2 +- .../V4Router_ExactOut3Hops_nativeOut.snap | 2 +- .../V4Router_ExactOutputSingle.snap | 2 +- ...r_ExactOutputSingle_nativeIn_sweepETH.snap | 2 +- .../V4Router_ExactOutputSingle_nativeOut.snap | 2 +- src/libraries/CalldataDecoder.sol | 103 ++++++++++-------- test/libraries/CalldataDecoder.t.sol | 47 ++++++++ test/mocks/MockCalldataDecoder.sol | 8 ++ 66 files changed, 178 insertions(+), 106 deletions(-) diff --git a/.forge-snapshots/BaseActionsRouter_mock10commands.snap b/.forge-snapshots/BaseActionsRouter_mock10commands.snap index 6f0ce725..8a065fc3 100644 --- a/.forge-snapshots/BaseActionsRouter_mock10commands.snap +++ b/.forge-snapshots/BaseActionsRouter_mock10commands.snap @@ -1 +1 @@ -59219 \ No newline at end of file +60677 \ No newline at end of file diff --git a/.forge-snapshots/Payments_swap_settleFromCaller_takeAllToMsgSender.snap b/.forge-snapshots/Payments_swap_settleFromCaller_takeAllToMsgSender.snap index 4620788d..2cd533ee 100644 --- a/.forge-snapshots/Payments_swap_settleFromCaller_takeAllToMsgSender.snap +++ b/.forge-snapshots/Payments_swap_settleFromCaller_takeAllToMsgSender.snap @@ -1 +1 @@ -129463 \ No newline at end of file +129854 \ No newline at end of file diff --git a/.forge-snapshots/Payments_swap_settleFromCaller_takeAllToSpecifiedAddress.snap b/.forge-snapshots/Payments_swap_settleFromCaller_takeAllToSpecifiedAddress.snap index da6562f3..89faf94c 100644 --- a/.forge-snapshots/Payments_swap_settleFromCaller_takeAllToSpecifiedAddress.snap +++ b/.forge-snapshots/Payments_swap_settleFromCaller_takeAllToSpecifiedAddress.snap @@ -1 +1 @@ -131381 \ No newline at end of file +131905 \ No newline at end of file diff --git a/.forge-snapshots/Payments_swap_settleWithBalance_takeAllToMsgSender.snap b/.forge-snapshots/Payments_swap_settleWithBalance_takeAllToMsgSender.snap index d9403287..55ac6b3a 100644 --- a/.forge-snapshots/Payments_swap_settleWithBalance_takeAllToMsgSender.snap +++ b/.forge-snapshots/Payments_swap_settleWithBalance_takeAllToMsgSender.snap @@ -1 +1 @@ -123586 \ No newline at end of file +124110 \ No newline at end of file diff --git a/.forge-snapshots/Payments_swap_settleWithBalance_takeAllToSpecifiedAddress.snap b/.forge-snapshots/Payments_swap_settleWithBalance_takeAllToSpecifiedAddress.snap index 0a690201..00e673a8 100644 --- a/.forge-snapshots/Payments_swap_settleWithBalance_takeAllToSpecifiedAddress.snap +++ b/.forge-snapshots/Payments_swap_settleWithBalance_takeAllToSpecifiedAddress.snap @@ -1 +1 @@ -123728 \ No newline at end of file +124252 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_burn_empty.snap b/.forge-snapshots/PositionManager_burn_empty.snap index 2a57f4b9..99bfcda6 100644 --- a/.forge-snapshots/PositionManager_burn_empty.snap +++ b/.forge-snapshots/PositionManager_burn_empty.snap @@ -1 +1 @@ -50181 \ No newline at end of file +50413 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_burn_empty_native.snap b/.forge-snapshots/PositionManager_burn_empty_native.snap index 2a57f4b9..99bfcda6 100644 --- a/.forge-snapshots/PositionManager_burn_empty_native.snap +++ b/.forge-snapshots/PositionManager_burn_empty_native.snap @@ -1 +1 @@ -50181 \ No newline at end of file +50413 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_burn_nonEmpty_native_withClose.snap b/.forge-snapshots/PositionManager_burn_nonEmpty_native_withClose.snap index b0561b5a..c04710dc 100644 --- a/.forge-snapshots/PositionManager_burn_nonEmpty_native_withClose.snap +++ b/.forge-snapshots/PositionManager_burn_nonEmpty_native_withClose.snap @@ -1 +1 @@ -125092 \ No newline at end of file +125551 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_burn_nonEmpty_native_withTakePair.snap b/.forge-snapshots/PositionManager_burn_nonEmpty_native_withTakePair.snap index 6e855474..6bb05428 100644 --- a/.forge-snapshots/PositionManager_burn_nonEmpty_native_withTakePair.snap +++ b/.forge-snapshots/PositionManager_burn_nonEmpty_native_withTakePair.snap @@ -1 +1 @@ -124652 \ No newline at end of file +124998 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_burn_nonEmpty_withClose.snap b/.forge-snapshots/PositionManager_burn_nonEmpty_withClose.snap index 77f3f994..78bd2544 100644 --- a/.forge-snapshots/PositionManager_burn_nonEmpty_withClose.snap +++ b/.forge-snapshots/PositionManager_burn_nonEmpty_withClose.snap @@ -1 +1 @@ -131944 \ No newline at end of file +132404 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_burn_nonEmpty_withTakePair.snap b/.forge-snapshots/PositionManager_burn_nonEmpty_withTakePair.snap index 5cf1ff9e..40ebeef5 100644 --- a/.forge-snapshots/PositionManager_burn_nonEmpty_withTakePair.snap +++ b/.forge-snapshots/PositionManager_burn_nonEmpty_withTakePair.snap @@ -1 +1 @@ -131505 \ No newline at end of file +131851 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_collect_native.snap b/.forge-snapshots/PositionManager_collect_native.snap index f2b569bb..5465c8d6 100644 --- a/.forge-snapshots/PositionManager_collect_native.snap +++ b/.forge-snapshots/PositionManager_collect_native.snap @@ -1 +1 @@ -145679 \ No newline at end of file +146253 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_collect_sameRange.snap b/.forge-snapshots/PositionManager_collect_sameRange.snap index da47bdd2..c4c3030a 100644 --- a/.forge-snapshots/PositionManager_collect_sameRange.snap +++ b/.forge-snapshots/PositionManager_collect_sameRange.snap @@ -1 +1 @@ -154245 \ No newline at end of file +154819 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_collect_withClose.snap b/.forge-snapshots/PositionManager_collect_withClose.snap index da47bdd2..c4c3030a 100644 --- a/.forge-snapshots/PositionManager_collect_withClose.snap +++ b/.forge-snapshots/PositionManager_collect_withClose.snap @@ -1 +1 @@ -154245 \ No newline at end of file +154819 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_collect_withTakePair.snap b/.forge-snapshots/PositionManager_collect_withTakePair.snap index f24c673c..a9a5e55a 100644 --- a/.forge-snapshots/PositionManager_collect_withTakePair.snap +++ b/.forge-snapshots/PositionManager_collect_withTakePair.snap @@ -1 +1 @@ -153708 \ No newline at end of file +154140 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap b/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap index 187ee7ef..a72b4a20 100644 --- a/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap +++ b/.forge-snapshots/PositionManager_decreaseLiquidity_native.snap @@ -1 +1 @@ -111488 \ No newline at end of file +111948 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decreaseLiquidity_withClose.snap b/.forge-snapshots/PositionManager_decreaseLiquidity_withClose.snap index e81fda0b..f829fc02 100644 --- a/.forge-snapshots/PositionManager_decreaseLiquidity_withClose.snap +++ b/.forge-snapshots/PositionManager_decreaseLiquidity_withClose.snap @@ -1 +1 @@ -119126 \ No newline at end of file +119700 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decreaseLiquidity_withTakePair.snap b/.forge-snapshots/PositionManager_decreaseLiquidity_withTakePair.snap index 5ececf3e..a08bab8c 100644 --- a/.forge-snapshots/PositionManager_decreaseLiquidity_withTakePair.snap +++ b/.forge-snapshots/PositionManager_decreaseLiquidity_withTakePair.snap @@ -1 +1 @@ -118589 \ No newline at end of file +119021 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decrease_burnEmpty.snap b/.forge-snapshots/PositionManager_decrease_burnEmpty.snap index 5661665a..630cc82d 100644 --- a/.forge-snapshots/PositionManager_decrease_burnEmpty.snap +++ b/.forge-snapshots/PositionManager_decrease_burnEmpty.snap @@ -1 +1 @@ -134633 \ No newline at end of file +135200 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decrease_burnEmpty_native.snap b/.forge-snapshots/PositionManager_decrease_burnEmpty_native.snap index c602072f..2d24dead 100644 --- a/.forge-snapshots/PositionManager_decrease_burnEmpty_native.snap +++ b/.forge-snapshots/PositionManager_decrease_burnEmpty_native.snap @@ -1 +1 @@ -127780 \ No newline at end of file +128348 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap b/.forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap index 91188f7d..c174df40 100644 --- a/.forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap +++ b/.forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap @@ -1 +1 @@ -131813 \ No newline at end of file +132387 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_decrease_take_take.snap b/.forge-snapshots/PositionManager_decrease_take_take.snap index 4f132ba3..387644ec 100644 --- a/.forge-snapshots/PositionManager_decrease_take_take.snap +++ b/.forge-snapshots/PositionManager_decrease_take_take.snap @@ -1 +1 @@ -119702 \ No newline at end of file +120276 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withClose.snap b/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withClose.snap index 62de932d..cfbe2e44 100644 --- a/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withClose.snap +++ b/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withClose.snap @@ -1 +1 @@ -158430 \ No newline at end of file +159004 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withSettlePair.snap b/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withSettlePair.snap index d9a4a319..c1180ea6 100644 --- a/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withSettlePair.snap +++ b/.forge-snapshots/PositionManager_increaseLiquidity_erc20_withSettlePair.snap @@ -1 +1 @@ -157512 \ No newline at end of file +157944 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increaseLiquidity_native.snap b/.forge-snapshots/PositionManager_increaseLiquidity_native.snap index 9c2ac001..24abd763 100644 --- a/.forge-snapshots/PositionManager_increaseLiquidity_native.snap +++ b/.forge-snapshots/PositionManager_increaseLiquidity_native.snap @@ -1 +1 @@ -140257 \ No newline at end of file +140831 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap b/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap index d3cf01fc..5f3bb304 100644 --- a/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap +++ b/.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap @@ -1 +1 @@ -136028 \ No newline at end of file +136318 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap b/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap index 32be09f4..8806511c 100644 --- a/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap +++ b/.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap @@ -1 +1 @@ -176737 \ No newline at end of file +177311 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_increase_autocompound_clearExcess.snap b/.forge-snapshots/PositionManager_increase_autocompound_clearExcess.snap index 2d2733e1..b48b941b 100644 --- a/.forge-snapshots/PositionManager_increase_autocompound_clearExcess.snap +++ b/.forge-snapshots/PositionManager_increase_autocompound_clearExcess.snap @@ -1 +1 @@ -147413 \ No newline at end of file +147987 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_native.snap b/.forge-snapshots/PositionManager_mint_native.snap index 84a0e5dc..497a2a91 100644 --- a/.forge-snapshots/PositionManager_mint_native.snap +++ b/.forge-snapshots/PositionManager_mint_native.snap @@ -1 +1 @@ -364121 \ No newline at end of file +364704 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_nativeWithSweep_withClose.snap b/.forge-snapshots/PositionManager_mint_nativeWithSweep_withClose.snap index 60f3f2c8..caa84c5d 100644 --- a/.forge-snapshots/PositionManager_mint_nativeWithSweep_withClose.snap +++ b/.forge-snapshots/PositionManager_mint_nativeWithSweep_withClose.snap @@ -1 +1 @@ -372502 \ No newline at end of file +373227 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_nativeWithSweep_withSettlePair.snap b/.forge-snapshots/PositionManager_mint_nativeWithSweep_withSettlePair.snap index 93cbab28..228cc541 100644 --- a/.forge-snapshots/PositionManager_mint_nativeWithSweep_withSettlePair.snap +++ b/.forge-snapshots/PositionManager_mint_nativeWithSweep_withSettlePair.snap @@ -1 +1 @@ -371867 \ No newline at end of file +372450 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_onSameTickLower.snap b/.forge-snapshots/PositionManager_mint_onSameTickLower.snap index bdfd8010..b808a0d9 100644 --- a/.forge-snapshots/PositionManager_mint_onSameTickLower.snap +++ b/.forge-snapshots/PositionManager_mint_onSameTickLower.snap @@ -1 +1 @@ -316969 \ No newline at end of file +317552 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_onSameTickUpper.snap b/.forge-snapshots/PositionManager_mint_onSameTickUpper.snap index e8242179..8e83229c 100644 --- a/.forge-snapshots/PositionManager_mint_onSameTickUpper.snap +++ b/.forge-snapshots/PositionManager_mint_onSameTickUpper.snap @@ -1 +1 @@ -317639 \ No newline at end of file +318222 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_sameRange.snap b/.forge-snapshots/PositionManager_mint_sameRange.snap index d280b4c8..5c2cca63 100644 --- a/.forge-snapshots/PositionManager_mint_sameRange.snap +++ b/.forge-snapshots/PositionManager_mint_sameRange.snap @@ -1 +1 @@ -243208 \ No newline at end of file +243791 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_settleWithBalance_sweep.snap b/.forge-snapshots/PositionManager_mint_settleWithBalance_sweep.snap index e9af2b9c..e4d3be42 100644 --- a/.forge-snapshots/PositionManager_mint_settleWithBalance_sweep.snap +++ b/.forge-snapshots/PositionManager_mint_settleWithBalance_sweep.snap @@ -1 +1 @@ -418116 \ No newline at end of file +418983 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_warmedPool_differentRange.snap b/.forge-snapshots/PositionManager_mint_warmedPool_differentRange.snap index 1929fba8..fa881598 100644 --- a/.forge-snapshots/PositionManager_mint_warmedPool_differentRange.snap +++ b/.forge-snapshots/PositionManager_mint_warmedPool_differentRange.snap @@ -1 +1 @@ -323000 \ No newline at end of file +323583 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_withClose.snap b/.forge-snapshots/PositionManager_mint_withClose.snap index 14f401cd..ac0d3107 100644 --- a/.forge-snapshots/PositionManager_mint_withClose.snap +++ b/.forge-snapshots/PositionManager_mint_withClose.snap @@ -1 +1 @@ -419522 \ No newline at end of file +420105 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_mint_withSettlePair.snap b/.forge-snapshots/PositionManager_mint_withSettlePair.snap index 1f3b16c9..4afca6a1 100644 --- a/.forge-snapshots/PositionManager_mint_withSettlePair.snap +++ b/.forge-snapshots/PositionManager_mint_withSettlePair.snap @@ -1 +1 @@ -418722 \ No newline at end of file +419163 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_multicall_initialize_mint.snap b/.forge-snapshots/PositionManager_multicall_initialize_mint.snap index dcbccb1f..9ed1b5dc 100644 --- a/.forge-snapshots/PositionManager_multicall_initialize_mint.snap +++ b/.forge-snapshots/PositionManager_multicall_initialize_mint.snap @@ -1 +1 @@ -463694 \ No newline at end of file +464277 \ No newline at end of file diff --git a/.forge-snapshots/V4Router_Bytecode.snap b/.forge-snapshots/V4Router_Bytecode.snap index 9c58b982..fb87573d 100644 --- a/.forge-snapshots/V4Router_Bytecode.snap +++ b/.forge-snapshots/V4Router_Bytecode.snap @@ -1 +1 @@ -7063 \ No newline at end of file +7148 \ No newline at end of file diff --git a/.forge-snapshots/V4Router_ExactIn1Hop_nativeIn.snap b/.forge-snapshots/V4Router_ExactIn1Hop_nativeIn.snap index b41c9104..bc9d9989 100644 --- a/.forge-snapshots/V4Router_ExactIn1Hop_nativeIn.snap +++ b/.forge-snapshots/V4Router_ExactIn1Hop_nativeIn.snap @@ -1 +1 @@ -115331 \ No newline at end of file +115722 \ No newline at end of file diff --git a/.forge-snapshots/V4Router_ExactIn1Hop_nativeOut.snap b/.forge-snapshots/V4Router_ExactIn1Hop_nativeOut.snap index ab96cecc..764f3bbb 100644 --- a/.forge-snapshots/V4Router_ExactIn1Hop_nativeOut.snap +++ b/.forge-snapshots/V4Router_ExactIn1Hop_nativeOut.snap @@ -1 +1 @@ -115652 \ No newline at end of file +116043 \ No newline at end of file diff --git a/.forge-snapshots/V4Router_ExactIn1Hop_oneForZero.snap b/.forge-snapshots/V4Router_ExactIn1Hop_oneForZero.snap index 73d5f844..40990e3c 100644 --- a/.forge-snapshots/V4Router_ExactIn1Hop_oneForZero.snap +++ b/.forge-snapshots/V4Router_ExactIn1Hop_oneForZero.snap @@ -1 +1 @@ -124470 \ No newline at end of file +124861 \ No newline at end of file diff --git a/.forge-snapshots/V4Router_ExactIn1Hop_zeroForOne.snap b/.forge-snapshots/V4Router_ExactIn1Hop_zeroForOne.snap index 7823d59e..a38c964a 100644 --- a/.forge-snapshots/V4Router_ExactIn1Hop_zeroForOne.snap +++ b/.forge-snapshots/V4Router_ExactIn1Hop_zeroForOne.snap @@ -1 +1 @@ -130193 \ No newline at end of file +130584 \ No newline at end of file diff --git a/.forge-snapshots/V4Router_ExactIn2Hops.snap b/.forge-snapshots/V4Router_ExactIn2Hops.snap index 5778e638..3b3ffc19 100644 --- a/.forge-snapshots/V4Router_ExactIn2Hops.snap +++ b/.forge-snapshots/V4Router_ExactIn2Hops.snap @@ -1 +1 @@ -179333 \ No newline at end of file +179724 \ No newline at end of file diff --git a/.forge-snapshots/V4Router_ExactIn2Hops_nativeIn.snap b/.forge-snapshots/V4Router_ExactIn2Hops_nativeIn.snap index c774d6ce..2862d64c 100644 --- a/.forge-snapshots/V4Router_ExactIn2Hops_nativeIn.snap +++ b/.forge-snapshots/V4Router_ExactIn2Hops_nativeIn.snap @@ -1 +1 @@ -170186 \ No newline at end of file +170577 \ No newline at end of file diff --git a/.forge-snapshots/V4Router_ExactIn3Hops.snap b/.forge-snapshots/V4Router_ExactIn3Hops.snap index e788c4f0..202b1b32 100644 --- a/.forge-snapshots/V4Router_ExactIn3Hops.snap +++ b/.forge-snapshots/V4Router_ExactIn3Hops.snap @@ -1 +1 @@ -228452 \ No newline at end of file +228843 \ No newline at end of file diff --git a/.forge-snapshots/V4Router_ExactIn3Hops_nativeIn.snap b/.forge-snapshots/V4Router_ExactIn3Hops_nativeIn.snap index 0eba0643..d3d293a5 100644 --- a/.forge-snapshots/V4Router_ExactIn3Hops_nativeIn.snap +++ b/.forge-snapshots/V4Router_ExactIn3Hops_nativeIn.snap @@ -1 +1 @@ -219329 \ No newline at end of file +219720 \ No newline at end of file diff --git a/.forge-snapshots/V4Router_ExactInputSingle.snap b/.forge-snapshots/V4Router_ExactInputSingle.snap index 4620788d..2cd533ee 100644 --- a/.forge-snapshots/V4Router_ExactInputSingle.snap +++ b/.forge-snapshots/V4Router_ExactInputSingle.snap @@ -1 +1 @@ -129463 \ No newline at end of file +129854 \ No newline at end of file diff --git a/.forge-snapshots/V4Router_ExactInputSingle_nativeIn.snap b/.forge-snapshots/V4Router_ExactInputSingle_nativeIn.snap index 830f1ed9..5e5c5b3b 100644 --- a/.forge-snapshots/V4Router_ExactInputSingle_nativeIn.snap +++ b/.forge-snapshots/V4Router_ExactInputSingle_nativeIn.snap @@ -1 +1 @@ -114601 \ No newline at end of file +114992 \ No newline at end of file diff --git a/.forge-snapshots/V4Router_ExactInputSingle_nativeOut.snap b/.forge-snapshots/V4Router_ExactInputSingle_nativeOut.snap index 11ac5a88..f36fa450 100644 --- a/.forge-snapshots/V4Router_ExactInputSingle_nativeOut.snap +++ b/.forge-snapshots/V4Router_ExactInputSingle_nativeOut.snap @@ -1 +1 @@ -114891 \ No newline at end of file +115282 \ No newline at end of file diff --git a/.forge-snapshots/V4Router_ExactOut1Hop_nativeIn_sweepETH.snap b/.forge-snapshots/V4Router_ExactOut1Hop_nativeIn_sweepETH.snap index a52548f6..9c6beb91 100644 --- a/.forge-snapshots/V4Router_ExactOut1Hop_nativeIn_sweepETH.snap +++ b/.forge-snapshots/V4Router_ExactOut1Hop_nativeIn_sweepETH.snap @@ -1 +1 @@ -121594 \ No newline at end of file +121985 \ No newline at end of file diff --git a/.forge-snapshots/V4Router_ExactOut1Hop_nativeOut.snap b/.forge-snapshots/V4Router_ExactOut1Hop_nativeOut.snap index 812d160f..adfd51ab 100644 --- a/.forge-snapshots/V4Router_ExactOut1Hop_nativeOut.snap +++ b/.forge-snapshots/V4Router_ExactOut1Hop_nativeOut.snap @@ -1 +1 @@ -116716 \ No newline at end of file +117107 \ No newline at end of file diff --git a/.forge-snapshots/V4Router_ExactOut1Hop_oneForZero.snap b/.forge-snapshots/V4Router_ExactOut1Hop_oneForZero.snap index 69c26da7..7692da73 100644 --- a/.forge-snapshots/V4Router_ExactOut1Hop_oneForZero.snap +++ b/.forge-snapshots/V4Router_ExactOut1Hop_oneForZero.snap @@ -1 +1 @@ -125534 \ No newline at end of file +125925 \ No newline at end of file diff --git a/.forge-snapshots/V4Router_ExactOut1Hop_zeroForOne.snap b/.forge-snapshots/V4Router_ExactOut1Hop_zeroForOne.snap index f3f87a2b..93703d01 100644 --- a/.forge-snapshots/V4Router_ExactOut1Hop_zeroForOne.snap +++ b/.forge-snapshots/V4Router_ExactOut1Hop_zeroForOne.snap @@ -1 +1 @@ -129479 \ No newline at end of file +129870 \ No newline at end of file diff --git a/.forge-snapshots/V4Router_ExactOut2Hops.snap b/.forge-snapshots/V4Router_ExactOut2Hops.snap index 217cc779..8e5c9c64 100644 --- a/.forge-snapshots/V4Router_ExactOut2Hops.snap +++ b/.forge-snapshots/V4Router_ExactOut2Hops.snap @@ -1 +1 @@ -179451 \ No newline at end of file +179842 \ No newline at end of file diff --git a/.forge-snapshots/V4Router_ExactOut2Hops_nativeIn.snap b/.forge-snapshots/V4Router_ExactOut2Hops_nativeIn.snap index b3ffcc15..f236e20d 100644 --- a/.forge-snapshots/V4Router_ExactOut2Hops_nativeIn.snap +++ b/.forge-snapshots/V4Router_ExactOut2Hops_nativeIn.snap @@ -1 +1 @@ -175511 \ No newline at end of file +175902 \ No newline at end of file diff --git a/.forge-snapshots/V4Router_ExactOut3Hops.snap b/.forge-snapshots/V4Router_ExactOut3Hops.snap index b9c05c9c..9449e574 100644 --- a/.forge-snapshots/V4Router_ExactOut3Hops.snap +++ b/.forge-snapshots/V4Router_ExactOut3Hops.snap @@ -1 +1 @@ -229430 \ No newline at end of file +229821 \ No newline at end of file diff --git a/.forge-snapshots/V4Router_ExactOut3Hops_nativeIn.snap b/.forge-snapshots/V4Router_ExactOut3Hops_nativeIn.snap index 8a2c58e1..c1ab906c 100644 --- a/.forge-snapshots/V4Router_ExactOut3Hops_nativeIn.snap +++ b/.forge-snapshots/V4Router_ExactOut3Hops_nativeIn.snap @@ -1 +1 @@ -225514 \ No newline at end of file +225905 \ No newline at end of file diff --git a/.forge-snapshots/V4Router_ExactOut3Hops_nativeOut.snap b/.forge-snapshots/V4Router_ExactOut3Hops_nativeOut.snap index 57d5c852..7eb77572 100644 --- a/.forge-snapshots/V4Router_ExactOut3Hops_nativeOut.snap +++ b/.forge-snapshots/V4Router_ExactOut3Hops_nativeOut.snap @@ -1 +1 @@ -220636 \ No newline at end of file +221027 \ No newline at end of file diff --git a/.forge-snapshots/V4Router_ExactOutputSingle.snap b/.forge-snapshots/V4Router_ExactOutputSingle.snap index 73769b96..5de03712 100644 --- a/.forge-snapshots/V4Router_ExactOutputSingle.snap +++ b/.forge-snapshots/V4Router_ExactOutputSingle.snap @@ -1 +1 @@ -128749 \ No newline at end of file +129140 \ No newline at end of file diff --git a/.forge-snapshots/V4Router_ExactOutputSingle_nativeIn_sweepETH.snap b/.forge-snapshots/V4Router_ExactOutputSingle_nativeIn_sweepETH.snap index fcfe8df3..6120543a 100644 --- a/.forge-snapshots/V4Router_ExactOutputSingle_nativeIn_sweepETH.snap +++ b/.forge-snapshots/V4Router_ExactOutputSingle_nativeIn_sweepETH.snap @@ -1 +1 @@ -120864 \ No newline at end of file +121255 \ No newline at end of file diff --git a/.forge-snapshots/V4Router_ExactOutputSingle_nativeOut.snap b/.forge-snapshots/V4Router_ExactOutputSingle_nativeOut.snap index 5a5a79b6..b7b122f5 100644 --- a/.forge-snapshots/V4Router_ExactOutputSingle_nativeOut.snap +++ b/.forge-snapshots/V4Router_ExactOutputSingle_nativeOut.snap @@ -1 +1 @@ -116061 \ No newline at end of file +116452 \ No newline at end of file diff --git a/src/libraries/CalldataDecoder.sol b/src/libraries/CalldataDecoder.sol index 5bcad8de..00cf6e33 100644 --- a/src/libraries/CalldataDecoder.sol +++ b/src/libraries/CalldataDecoder.sol @@ -11,35 +11,61 @@ library CalldataDecoder { error SliceOutOfBounds(); - /// @notice equivalent to SliceOutOfBounds.selector - bytes4 constant SLICE_ERROR_SELECTOR = 0x3b99b53d; + /// @notice mask used for offsets and lengths to ensure no overflow + /// @dev no sane abi encoding will pass in an offset or length greater than type(uint32).max + /// (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_AND_WORD_ALIGN = 0xffffffe0; - /// @dev equivalent to: abi.decode(params, (bytes, bytes[])) in calldata + /// @notice equivalent to SliceOutOfBounds.selector, stored in least-significant bits + uint256 constant SLICE_ERROR_SELECTOR = 0x3b99b53d; + + /// @dev equivalent to: abi.decode(params, (bytes, bytes[])) in calldata (requires strict abi encoding) function decodeActionsRouterParams(bytes calldata _bytes) internal pure returns (bytes calldata actions, bytes[] calldata params) { assembly ("memory-safe") { - // The offset of the 0th element is 0, which stores the offset of the length pointer of actions array. - // The offset of the 1st element is 32, which stores the offset of the length pointer of params array. - let actionsPtr := add(_bytes.offset, calldataload(_bytes.offset)) - let paramsPtr := add(_bytes.offset, calldataload(add(_bytes.offset, 0x20))) + // Strict encoding requires that the data begin with: + // 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) - // The length is stored as the first element - actions.length := calldataload(actionsPtr) - params.length := calldataload(paramsPtr) + // 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) - // The actual data is stored in the slot after the length - actions.offset := add(actionsPtr, 0x20) - params.offset := add(paramsPtr, 0x20) + // 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 - // Calculate how far `params` is into the provided bytes - let relativeOffset := sub(params.offset, _bytes.offset) - // Check that that isn't longer than the bytes themselves, or revert - if lt(_bytes.length, add(params.length, relativeOffset)) { + for { let offset := 0 } lt(offset, tailOffset) { offset := add(offset, 32) } { + 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(itemLengthPointer), 0x1f), OFFSET_OR_LENGTH_MASK_AND_WORD_ALIGN), 0x20) + expectedOffset := add(expectedOffset, length) + } + + // 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(0, 0x04) + revert(0x1c, 4) } } } @@ -228,38 +254,29 @@ library CalldataDecoder { } } - /// @notice Decode the `_arg`-th element in `_bytes` as a dynamic array - /// @dev The decoding of `length` and `offset` is universal, - /// whereas the type declaration of `res` instructs the compiler how to read it. - /// @param _bytes The input bytes string to slice - /// @param _arg The index of the argument to extract - /// @return length Length of the array - /// @return offset Pointer to the data part of the array - function toLengthOffset(bytes calldata _bytes, uint256 _arg) - internal - pure - returns (uint256 length, uint256 offset) - { - uint256 relativeOffset; - assembly ("memory-safe") { - // The offset of the `_arg`-th element is `32 * arg`, which stores the offset of the length pointer. - // shl(5, x) is equivalent to mul(32, x) - let lengthPtr := add(_bytes.offset, calldataload(add(_bytes.offset, shl(5, _arg)))) - length := calldataload(lengthPtr) - offset := add(lengthPtr, 0x20) - relativeOffset := sub(offset, _bytes.offset) - } - if (_bytes.length < length + relativeOffset) revert SliceOutOfBounds(); - } - /// @notice Decode the `_arg`-th element in `_bytes` as `bytes` /// @param _bytes The input bytes string to extract a bytes string from /// @param _arg The index of the argument to extract function toBytes(bytes calldata _bytes, uint256 _arg) internal pure returns (bytes calldata res) { - (uint256 length, uint256 offset) = toLengthOffset(_bytes, _arg); + uint256 length; assembly ("memory-safe") { + // The offset of the `_arg`-th element is `32 * arg`, which stores the offset of the length pointer. + // shl(5, x) is equivalent to mul(32, x) + let lengthPtr := + add(_bytes.offset, and(calldataload(add(_bytes.offset, shl(5, _arg))), OFFSET_OR_LENGTH_MASK)) + // the number of bytes in the bytes string + length := and(calldataload(lengthPtr), OFFSET_OR_LENGTH_MASK) + // the offset where the bytes string begins + let offset := add(lengthPtr, 0x20) + // assign the return parameters res.length := length res.offset := offset + + // if the provided bytes string isnt as long as the encoding says, revert + if lt(add(_bytes.length, _bytes.offset), add(length, offset)) { + mstore(0, SLICE_ERROR_SELECTOR) + revert(0x1c, 4) + } } } } diff --git a/test/libraries/CalldataDecoder.t.sol b/test/libraries/CalldataDecoder.t.sol index da24aa95..0a3fc737 100644 --- a/test/libraries/CalldataDecoder.t.sol +++ b/test/libraries/CalldataDecoder.t.sol @@ -9,6 +9,7 @@ import {MockCalldataDecoder} from "../mocks/MockCalldataDecoder.sol"; import {PositionConfig} from "../shared/PositionConfig.sol"; import {IV4Router} from "../../src/interfaces/IV4Router.sol"; import {PathKey} from "../../src/libraries/PathKey.sol"; +import {CalldataDecoder} from "../../src/libraries/CalldataDecoder.sol"; contract CalldataDecoderTest is Test { MockCalldataDecoder decoder; @@ -147,6 +148,52 @@ contract CalldataDecoderTest is Test { assertEq(Currency.unwrap(currency), Currency.unwrap(_currency)); } + function test_fuzz_decodeActionsRouterParams(bytes memory _actions, bytes[] memory _actionParams) public view { + bytes memory params = abi.encode(_actions, _actionParams); + (bytes memory actions, bytes[] memory actionParams) = decoder.decodeActionsRouterParams(params); + + assertEq(actions, _actions); + for (uint256 i = 0; i < _actionParams.length; i++) { + assertEq(actionParams[i], _actionParams[i]); + } + } + + function test_decodeActionsRouterParams_sliceOutOfBounds() public { + // create actions and parameters + bytes memory _actions = hex"12345678"; + bytes[] memory _actionParams = new bytes[](4); + _actionParams[0] = hex"11111111"; + _actionParams[1] = hex"22"; + _actionParams[2] = hex"3333333333333333"; + _actionParams[3] = hex"4444444444444444444444444444444444444444444444444444444444444444"; + + bytes memory params = abi.encode(_actions, _actionParams); + + bytes memory invalidParams = new bytes(params.length - 1); + // dont copy the final byte + for (uint256 i = 0; i < params.length - 2; i++) { + invalidParams[i] = params[i]; + } + + assertEq(invalidParams.length, params.length - 1); + + vm.expectRevert(CalldataDecoder.SliceOutOfBounds.selector); + decoder.decodeActionsRouterParams(invalidParams); + } + + function test_decodeActionsRouterParams_emptyParams() public view { + // create actions and parameters + bytes memory _actions = hex""; + bytes[] memory _actionParams = new bytes[](0); + + bytes memory params = abi.encode(_actions, _actionParams); + + (bytes memory actions, bytes[] memory actionParams) = decoder.decodeActionsRouterParams(params); + assertEq(actions, _actions); + assertEq(actionParams.length, _actionParams.length); + assertEq(actionParams.length, 0); + } + function test_fuzz_decodeCurrencyPair(Currency _currency0, Currency _currency1) public view { bytes memory params = abi.encode(_currency0, _currency1); (Currency currency0, Currency currency1) = decoder.decodeCurrencyPair(params); diff --git a/test/mocks/MockCalldataDecoder.sol b/test/mocks/MockCalldataDecoder.sol index d5edbb64..e25d8ec7 100644 --- a/test/mocks/MockCalldataDecoder.sol +++ b/test/mocks/MockCalldataDecoder.sol @@ -22,6 +22,14 @@ contract MockCalldataDecoder { bytes hookData; } + function decodeActionsRouterParams(bytes calldata params) + external + pure + returns (bytes calldata actions, bytes[] calldata actionParams) + { + return params.decodeActionsRouterParams(); + } + function decodeModifyLiquidityParams(bytes calldata params) external pure