Repository navigation
fix: OZ 2.3.0 audit fixes (L-01, N-02, N-03, N-04, N-05, N-06, N-07) - #523
Merged
Merged
Conversation
…tion gate A sub-plan flagged allow-revert that tripped NestedExecutionNotPermitted was reported as an ordinary failure, so a plain execute inside a foreign unlock could wrap its V4_SWAP in such a sub-plan, skip the swap silently, and leave the route's input in the router. The gate is a consent check, not a route failure: when a sub-plan fails with exactly that selector the Dispatcher re-throws it. OZ 2.3.0 audit L-01.
The note said a donation drives the price below minHopPriceX36 and reverts, and that skim() returns it. A donation only reverts the route when it trips the bound, otherwise it is swapped along with the input, and skim(to) sends excess to whoever calls it rather than back to the donor. Wording now matches the V3 note. OZ 2.3.0 audit N-02.
…ut behind Since #584 a route whose hook funds the swap input succeeds without touching what the plan pre-funded the router with. Document that plans funding the router must end with a full-balance return, since any balance left in the router can be swept by anyone. OZ 2.3.0 audit N-05.
toLengthOffset checked the head word with add(32 * _arg, 32) against the slice length. For _arg >= 2^251 the multiplication wraps, the check passes, and the read lands on a different element. Every caller passes a small constant today, so this is defensive, but the wrap-free comparison costs a few bytes and removes the assumption. OZ 2.3.0 audit N-06.
execute has always had a deadline-free form for composers that enforce their own deadline, but executeNested only shipped with the deadline variant, so contracts opting into a shared unlock had to pass a dummy deadline. Both variants now share one private body. OZ 2.3.0 audit N-03.
v2SwapExactOutput computed the input from reserves, paid the first pair and ran the hops without ever checking what reached the recipient, so a fee-on-transfer or withholding token at any hop let the route succeed on a shortfall, uncapped for a token that keeps what it is sent. The recipient balance check that exact input already performed now lives in the hop loop and both entrypoints pass their bound: exact output passes the requested amount itself. Sharing the two balance reads makes the fix cheaper than the check it replaces. OZ 2.3.0 audit N-04.
The flag description read as if any command could be allowed to revert. Only commands built on an external call report failure through it; swaps, transfers, wraps, sweeps and the Across deposit are internal calls and revert the transaction regardless, so the README now lists the commands the flag covers, points at EXECUTE_SUB_PLAN for the rest, and notes that the nested-execution gate is never swallowed. OZ 2.3.0 audit N-07.
Picks up EmptyPath on the multi-hop swaps (L-02), the corrected exact-output comment (N-01), the head-word bound in CalldataDecoder.toBytes (N-08) and the V4Quoter fixes from v4-periphery #588 (N-05), at 02ffbe93.
skim(to) pays whoever calls it first, which can be the donor, so "not back to the donor" overstated it. The note also now covers a disabled per-hop bound, where a donation is always swapped along with the input. OZ 2.3.0 audit N-02. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
hensha256
reviewed
Oct 1, 2026
…l case Any exact-output swap paid from the router's own balance spends only what the route costs, so the unspent pre-fund stays in the router whether or not a hook is involved; a hook that funds the input is just the zero-cost extreme. The note now states the sweep as the general rule, names the payerIsUser = false and native-input cases OZ called out, and drops TRANSFER from the funding list since it pays out of the router. OZ 2.3.0 audit N-05. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Fixes for OpenZeppelin's Universal Router v2.3.0 audit (report dated 2026-09-21), one commit per finding, on
dev-2.3.0. The periphery-side findings (L-02, N-01, N-05 quoter, N-08) live in v4-periphery PR (link), which this PR pins.NestedExecutionNotPermittedis re-thrown even underFLAG_ALLOW_REVERT, so the gate cannot be turned into a silent skip that strands the route's inputskim(to)lets whoever calls it first claim the excess, not necessarily the donorexecuteNestedoverload, sharing one private body with the deadline formpayerIsUser = falseor a native input) leaves the unspent pre-fund in the router, so such plans must end with a full-balance return; a hook that funds the input is the zero-cost extremeBytesLib.toLengthOffsetcompares its head-word guard in whole words, so a huge index cannot wrap past itFLAG_ALLOW_REVERTaffects, points atEXECUTE_SUB_PLANfor the rest, and notes the nested gate is never swallowedBytecode
dev-2.3.0beforePeriphery L-02 and N-08 together cost 72 bytes on the router, L-01 149, N-06 8. N-03 and N-04 net 19 together, because moving the exact-input delivery check into the shared loop paid for most of the exact-output check.
Testing
executeNestedinside an unlock; v2 exact output with a fee-on-transfer output token, a fee-on-transfer intermediate token, and a withholding token, plus the standard-token control;toLengthOffsetwrapping index and out-of-bounds fuzz.forge test --isolatewithFORK_URL: 186 passed, 0 failed.yarn test:hardhatwithFORK_URL: see the snapshot commit.Before merging
Re-pin
lib/v4-peripheryto the squash commit of the periphery PR ondev-ur-2.3.0once it lands, as #508 did.AI-Generated Description
What
Fixes for OpenZeppelin's Universal Router v2.3.0 audit (report dated 2026-09-21), one commit per finding, on
dev-2.3.0. The periphery-side findings (L-02, N-01, N-05 quoter, N-08) live in v4-periphery PR, which this PR pins.NestedExecutionNotPermittedis re-thrown even underFLAG_ALLOW_REVERT, so the gate cannot be turned into a silent skip that strands the route's inputskim(to)lets whoever calls it first claim the excess, not necessarily the donorexecuteNestedoverload, sharing one private body with the deadline formpayerIsUser = falseor a native input) leaves the unspent pre-fund in the router, so such plans must end with a full-balance return; a hook that funds the input is the zero-cost extremeBytesLib.toLengthOffsetcompares its head-word guard in whole words, so a huge index cannot wrap past itFLAG_ALLOW_REVERTaffects, points atEXECUTE_SUB_PLANfor the rest, and notes the nested gate is never swallowedChanges
Dispatcher.sol: After anEXECUTE_SUB_PLANcall, check whether the revert isNestedExecutionNotPermittedand re-throw it regardless ofFLAG_ALLOW_REVERT(L-01)UniversalRouter.sol/IUniversalRouter.sol: Add a deadline-freeexecuteNested(commands, inputs)overload; both overloads now delegate to a shared_executeNestedprivate function (N-03)V2SwapRouter.sol: Move the delivery check (balanceBefore/balanceOfcomparison) into_v2Swapso both exact-input and exact-output paths share it; exact-output passesamountOutas the minimum (N-04)BytesLib.sol: Replace the byte-offset overflow-prone guardgt(add(headOffset, 0x20), _bytes.length)with the whole-word comparisoniszero(gt(div(_bytes.length, 0x20), _arg))(N-06)README.md: Correct the V2 donation note (N-02), document hook-funded exact-output input retention (N-05), list which commandsFLAG_ALLOW_REVERTaffects (N-07)lib/v4-periphery: Bumped to02ffbe93(the OZ 2.3.0 fix branch)Bytecode
dev-2.3.0beforeTesting
executeNestedinside an unlock; v2 exact output with a fee-on-transfer output token, a fee-on-transfer intermediate token, and a withholding token, plus the standard-token control;toLengthOffsetwrapping index and out-of-bounds fuzzforge test --isolatewithFORK_URL: 186 passed, 0 failedyarn test:hardhatwithFORK_URL: see the snapshot commitBefore merging
Re-pin
lib/v4-peripheryto the squash commit of the periphery PR ondev-ur-2.3.0once it lands, as #508 did.