Internal reviews

Four reviews of UvRouter before it went live, with every finding and what I did about it.

About
Review A
Review B
Review C
Review D
Tests and tools
Security notes

About these reviews

No outside firm has audited UvRouter. I reviewed it myself in four passes. Reviews A and B were two independent full reviews, started from different prompts: one was given only the source and the threat model, the other the code plus well-known router-drain post-mortems, without my conclusions. Each wrote its own tests and proofs of concept. Reviews C and D were targeted at one change each. After A and B I fixed the findings with a regression test for each, and both reviewers re-checked the fixes.

All four finished before the contract was deployed. Where a finding describes a state of the code or the build, that state was found and fixed before deployment. The deployed contract is the one on the security page. Severities are the reviewers' own.

Review	Scope	Medium	Low	Info
A	Contract, deploy scripts, tests and the API's transaction builder	1	5	9
B	Contract and builder, eight properties	0	1	4
C	The per-swap signed fee and the build settings	0	1	6
D	The allowance entry point and the API code around it	0	1	3

Review D found nothing in the contract itself. Its Low finding and one Info item were in the API.

Review A

A full review of the contract, the deploy scripts, the tests and the Rust code that builds transactions. Verdict: no way found for anyone other than the signer to move a user's funds, redirect output, replay or re-target a signature, or make a fake pool pull tokens. The one Medium is not a contract bug. It is how much a compromised or buggy API can take from a user who signs, because the route was signed as an opaque hash and minOut as a raw integer.

What held

The Permit2 type string and witness hash are correct, and all swap fields are bound by the signature, the Permit2 hash or the owner check.
The nonce is single use, the spender binds the deployment and the chain id is in Permit2's domain. A front-runner can only execute the signed swap unchanged.
The venue constants match the chain. Locally derived CREATE2 addresses equal getPair and getPool for WBNB and USDT on PancakeSwap V2, Uniswap V2, PancakeSwap V3 (all four fee tiers) and Uniswap V3 (all four fee tiers).
All seven capped tokens report 18 decimals on chain, so the caps are in the right units.
The V3 callback pays only the pool being swapped, only what it owes, and only once. A hooked token cannot make a pool call back twice.
Reentrancy, minOut on the recipient's balance, the native path and the caps (the capped quantity is the input token, so multi-hop and native versus wrapped cannot bypass it) all held.

Findings

ID	Severity	Finding	Outcome
A-06	Medium	The route is signed as an opaque hash and `minOut` as a raw integer. A compromised API could return `minOut = 1` and a hop through an attacker-seeded pool and take a swap, up to one cap per signature.	Reduced and documented. Hops are now a typed array that wallets display. Recipient, output token and `minOut` were already readable. The residual risk is on the security page.
A-01	Low	The fee could be skipped. A last hop into a self-made dust pool returned the real output as a fee-free refund. A fork proof of concept paid no fee on a 612 USDT swap.	Fixed. A leftover intermediate token pays the swap's fee on its refund, and a path may not return to the input token. The same proof of concept now pays the fee and the user gets no more.
A-07	Low	`minOut` could sit further under the real output than the stated slippage, if the engine's dry run charged a fee in the sell token.	Fixed in the API. `minOut` comes from the route's raw output for the whole sell amount, and no transaction is built without it or with a solver fee.
A-12	Low	The API never checked its config against the deployed contract, so a wrong contract address would have made users sign a permit to an arbitrary spender.	Fixed. The API reads the contract back at startup and answers 404 on `/v1/swap` until everything matches, retrying every minute. The deploy script runs the same read-back.
A-16	Low	A 65-byte signature placeholder excluded contract wallets and EIP-7702 accounts, which would revert.	Fixed. A signature of any length is spliced into the calldata.
A-04	Low	A lifted signature can be executed first, which reverts the transaction that carried it. The swap runs as signed, but the executor picks the moment.	Documented for integrators. The parameters are the same, the outcome may not be.
A-02	Info	Any token sitting in the router goes to the next caller.	Accepted, documented.
A-03	Info	The fee rounds to zero on tiny outputs.	Accepted, documented.
A-05	Info	`Swapped` can be emitted with self-made tokens.	Accepted. Accounting must filter on the verified token list.
A-08	Info	Thin V3 pools with a small tick spacing can burn a lot of gas.	Accepted. Only the submitter pays.
A-09	Info	Force-sent BNB is stuck.	Accepted. Nobody can take it.
A-10	Info	A Solidity compiler bug affecting transient storage clearing under via-IR in 0.8.28 to 0.8.33.	The compiler pin moved to 0.8.38. See review C, which found the build was via-IR after all.
A-11	Info	The fee recipient came from an environment variable with no check and cannot be rotated.	Fixed. It is a constant in the deploy config, and the script reads every immutable, venue and cap back.
A-13	Info	The gas estimate is a fixed table and can under-estimate.	Fixed with 25 percent headroom on the gas limit.
A-15	Info	Generated nonces could collide across API processes.	Fixed with a random per-process salt in the nonce's top bits.

Review A also corrected my own notes in several places: the API does fill in the recipient, front-running gives the same parameters and not the same outcome, intermediate tokens are refunded, and the per-swap bound holds per call and not per transaction. Not covered: the engine's internals and the token prices behind the cap sizes.

Review B

A property-by-property review with 29 tests of its own, including fuzz runs and five fork tests. Seven of eight properties held and one failed on an edge case.

Property	Result
The signature binds exactly the signed swap, with no replay and no third-party change	Holds. Fourteen fields were each changed in turn (output token, `minOut`, recipient, unwrap flag, each hop field, hop count, amount, token, nonce, deadline, owner) and each change was rejected. Dirty high bits in calldata are rejected.
Funds go only to derived pools, and the callback pays only the live pool, only what it owes, once	Holds, including a token hook calling the callback during a live swap. No other code can sit at a derived PancakeSwap V3 address.
The recipient receives at least `minOut` by balance	Holds, including taxed outputs and a recipient that is a pool, the fee recipient or the token.
Reentrancy	Holds from the output token, the input token and the unwrap recipient.
The router holds no user funds after a call	Failed on an edge case, finding B-1.
Caps and fee math	Holds. Native and WBNB share one cap, unlisted tokens are refused, and the fee is bounded and rounds down.
Native BNB in and out, and stray BNB	Holds. Forced BNB cannot be extracted.
The API's builder matches the contract	Holds. Six cases built by the real builder match the contract for the witness hash, the Permit2 digest and both calldata forms. A signature made by `cast` over the typed-data JSON recovers against the contract's digest.

Findings

ID	Severity	Finding	Outcome
B-1	Low	A rebasing or reflection output token left a residue in the router. The contract refunded the input and the intermediates but never the output token, so with a 0.1 percent rebase per transfer about 0.1 percent of the output stayed behind, for the next swap to collect.	Fixed. The output token is swept to the payer after delivery. A regression test and an invariant that the router is empty after any route cover it.
I-1	Info	The wallet showed the route only as an opaque hash.	Fixed with A-06.
I-2	Info	Caps limit the signed input of one swap, not total volume or donated tokens.	By design, documented.
I-3	Info	Forced BNB is stuck.	Same as A-09.
I-4	Info	Nothing checked that the API's fee setting equalled the contract's.	Fixed with A-12.

Review C: the per-swap fee

After reviews A and B I changed the fee from one constant fixed at construction to a rate per swap, signed in the Permit2 witness and passed as an argument on the native path. Review C covered that change, the fee engine in the API, and the compiler pin. It found no way for anyone but the signer to change a swap's fee, to charge above MAX_FEE_BPS, or to use the refund paths against a user. The full suite, with fork tests, passed 115 of 115 at that point.

What the review checked included: type strings rebuilt from their parts; wallet typed data hashed by forge's independent EIP-712 code and executed on the real Permit2; a submitter trying every other fee value (each failed with an invalid signer or FeeTooHigh, and the signature stayed usable); contract wallets; FeeTooHigh before any external call, shown by state-diff recording; fee math for output tokens with 0 to 24 decimals; minOut measured after the fee; two stalled intermediate hops; and the native path. In the API, four independent bounds hold the fee: config validation, a serve-time clamp on every table entry, a check in the transaction builder, and the contract's own limit. No request parameter reaches the fee.

ID	Severity	Finding	Outcome
C-1	Low	UvRouter was built through via-IR by accident, the opposite of what my notes said. My build configuration restricted a Permit2 dependency to via-IR, and forge compiles every file that imports a restricted file under the restricted profile, so UvRouter's import of a Permit2 interface pulled UvRouter into it. On solc 0.8.28 that build met the conditions of a known high-severity transient-storage bug. With 0.8.38 no known bug applies in either pipeline, but via-IR on the newest compiler was a less conservative choice than I had documented.	Fixed before deployment. Only two files that need via-IR are restricted, UvRouter builds through the legacy pipeline, and a test fails if the compiled metadata says `viaIR`. The suite passed on both builds. Nothing was deployed with the earlier configuration.
C-2	Info	solc 0.8.38 was 23 days old. An empty bug list on a new release partly means nobody has looked yet.	Kept 0.8.38 with the legacy pipeline, and re-checked the bug list on deploy day.
C-3	Info	A caller can sign fee 0. The only risk is lost revenue. Accounting must read the `fee` field of `Swapped`, which excludes fees on intermediate refunds.	Documented as a design choice.
C-4	Info	The fee is the API's choice up to 50 bps. The wallet shows a bare `feeBps` number, and on the native path the fee is in calldata the wallet does not decode.	Bounded by the immutable limit and covered by the compromised-API risk. Signing screens should print the fee as a percentage.
C-5	Info	The fee rounds to zero below 10,000 / `feeBps` base units of output.	Accepted. Negligible for 18-decimal tokens.
C-6	Info	The fee depends on pair, class and size bucket, so splitting an order can reach a cheaper bucket, and quote and swap can differ by one bucket at an edge. Bad input data to the fee table can push fees only up to the table's maximum of 30 bps, never past the contract's 50.	Accepted. Each response carries the rate that is signed.
C-7	Info	My notes said the build was legacy.	Corrected with C-1.

Review D: the allowance path

Before deployment I added swapWithAllowance, so smart wallets and aggregators can pay an ERC-20 from their own balance with approve and transferFrom. Review D covered it with five questions. The contract change held on all five and the review found nothing in the contract. The full suite, including fork tests, passed 155 of 155.

Nobody can spend an allowance that is not the caller's. A test recorded every call the router makes, with a bystander's unlimited approval standing, on all entry points. transferFrom appears exactly once, only on swapWithAllowance, with from equal to the caller and the destination equal to the first stop. A malicious input token that fired eleven different payloads, including re-entry and a Permit2 call, reached nothing.
The pull is exact and goes to the same first stop as the Permit2 path. A differential test ran the same swap through both paths from one snapshot, across plain and fee-on-transfer inputs, V2, V3 and mixed routes, half-filled hops and every fee from 0 to the maximum plus one, and got the same return value, the same revert data and the same balances.
The fee bound, minOut after the fee, the deadline and the caps all behave as on the other paths.
On the BNB Chain deployment only the seven capped tokens are accepted as input, and any other token is refused before any external call.
The API side held, apart from the items below.

ID	Severity	Finding	Outcome
D-1	Low	The API checked the input cap after calling the pricing engine. An over-cap request still never got a transaction, but it cost an engine call and could hide the cap error behind an engine error.	Fixed. The cap is checked before the engine is called.
D-2	Info	The API's ERC-20 mode was not yet reachable from the server. It was safe, since such a request returned an error, and the code that would choose the mode per request had not been written.	Wired in before launch, with a version gate on the contract, automatic mode selection from the taker's code, and the cap check first.
D-3	Info	With an integrator as the caller, refunds stop at the integrator. A generic-call integrator that max-approves can have its own stray balances spent by its next caller. That is the integrator's surface, as with any DEX.	No contract change. Stated for integrators on the security page.
D-4	Info	Two statements in my notes were too strong: that an aggregator approves per call, which a max-approving integrator does not, and that the caps bound the loss from a hypothetical `from` bug on standing approvals, which they do not.	Corrected. The real limit is exact approvals, which the API already returns.

The review also confirmed that a comment cleanup I made in the same change altered no bytecode, by compiling the source with and without the five code additions and comparing hashes. Not covered: the Slither rerun, the pricing engine, and the integrators' own contracts.

Tests and tools

The suite covers unit tests, attack scenarios, each review's own tests, fuzz tests at 1,000 runs each, and four invariants run at 256 runs of depth 32. The invariants are that the router holds nothing after a swap, that only the owner's own signatures spend, that no attack mutation of a signed swap succeeds, and that the fee equals exactly the signed rate. Everything except the fork tests passes without a network.
Fork tests run real WBNB, USDT and CAKE routes through all four venues. Each output equals PancakeSwap's router getAmountsOut or QuoterV2 (or the Uniswap V2 formula) minus my fee, and the CREATE2 addresses equal the factories' getPair and getPool.
Rust and Solidity agree byte for byte. For several cases, the typed-data JSON the API hands to wallets, hashed by forge's own EIP-712 implementation, equals the digest built from the contract's witnessHash and the real Permit2 domain, and the calldata hashes match.
Slither 0.11.6 was run after the per-swap fee change and again for the allowance entry point. Every result is triaged and none is open. The ones that matter were the unwrap-to-recipient send (by design, the recipient is signed by the payer), the recipient balance read before the output transfer (this is the balance-delta check itself, under the lock), writes after external calls (every entry is under the transient lock and the callback is gated by _expectedPool), and calls in a loop (at most 4 hops). The rest were false positives or informational.

Back to the security notes. To report a problem, message me on Telegram, @uvstarr1.