# Audit report > Audit the contracts in kadiri-seer/quietpay as they are. | | | |---|---| | Repository | https://github.com/kadiri-seer/quietpay.git | | Commit | `095e65a919d9526d47a53f1fbddc23fa5ca61399` | | Job | `79f6731a-835e-4df4-90bf-d07c0ba201c5` | | Judged | 2026-11-04 07:43 UTC | | Findings | 2 medium · 3 low · 2 info | Four agents audited the code as it is at `095e65a`, each in one area (math, permissions, economics, control flow), and a judge reproduced, merged and ranked what they found, then read the code once more itself. Nothing in the repository was changed or deployed. ## Findings ### 1. Medium: QuietPayRouterV2.payOnce lets any link holder burn a one-time link for one base unit paid to themselves `contracts/src/QuietPayRouterV2.sol:98` ``` paidOnce[requestId] = true; ``` Merged from four specialist reports (economics, math, permissions, flow) that describe the same mechanism. payOnce (lines 86-100) is callable by anyone and the only state it records is paidOnce[requestId] = true (line 98); line 94 then refuses every later payOnce for that ID. Nothing binds the request ID to the recipient (the stealth address and ephemeral key are caller-chosen and cannot be checked onchain, since the stealth meta-address exists only inside the link) or to the invoice amount (`amount` only has to be non-zero and equal the amount field in the caller's own metadata, line 105). The 16-byte request ID is bytes 47..62 of every payment link (src/lib/link.ts:11), so everyone the link was shown to knows it. Such a party calls payOnce with the ID, a second address they control as the stealth address, amount 1, any 33-byte key starting 0x02/0x03 and self-consistent 76-byte metadata. All checks pass, the unit lands in their own address, and the ID is consumed. The honest payer's payOnce then reverts AlreadyPaid. The app makes this authoritative: src/lib/payer.ts:290-298 reads paidOnce and fails preflight with 'This link was already paid' in every mode, and submitRoutedPayment (payer.ts:560-564) has no other path for fixed links. The recipient's scanner never matches the griefer's announcement (src/lib/receive.ts:36 requires the ephemeral key to derive to their keys), so the recipient sees nothing and is never paid. Before v2 the app refused to count an announcement unless the one-time address had received at least the link's amount (SECURITY.md:158-162, payer.ts:69-73, 'Faking that means paying in full'); the onchain flag reintroduces the weaker rule at a cost of one base unit plus gas. No funds are lost; the impact is a cheap targeted denial of service on any one-time invoice by anyone holding the link, leaving the payer with a dead-end error and the recipient unpaid until they notice and issue a new link. Fix that preserves the one-time design: record what was paid instead of a bool (for example mapping(bytes16 => uint256) paidAmount, rejecting a payOnce only when an earlier payment of at least this call's amount was recorded, or keying the record on keccak256(requestId, amount)); then a sub-amount self-payment cannot consume the ID and burning costs the full invoice amount, which is the floor without recipient involvement. In the app, treat the onchain record as 'paid' only when the recorded amount is at least the link amount, and warn rather than hard-fail otherwise. Reproduced by reading the code; the attached proof test was reviewed line by line but could not be executed because this session's shell sandbox failed to initialize, so forge did not run here. **Reproduction** Deploy QuietPayTestUSDG T, ERC5564Announcer A, QuietPayRouterV2 R = new QuietPayRouterV2(T, A). Recipient issues a fixed 7 tUSDG link with requestId R1 = 0x0102030405060708090a0b0c0d0e0f10. Attacker (any address that has seen the link, with a second address SINK): T.faucet(1); T.approve(R, 1); R.payOnce(R1, SINK, 1, 0x02 || 32 bytes, meta1) where meta1 = 0x42 || 0xa9059cbb || bytes20(T) || uint256(1) || 0x515001 || R1 (76 bytes). Actual: succeeds, R.paidOnce(R1) == true, T.balanceOf(SINK) == 1, Announcement emitted for SINK. Honest payer: T.faucet(7_000_000); T.approve(R, 7_000_000); R.payOnce(R1, recipientStealth, 7_000_000, key, meta7) with uint256(7_000_000) in bytes 25..56. Expected (contract header lines 22-23 and the pre-v2 rule in SECURITY.md:158-162): the invoice is still payable because no payment of the link's amount reached the recipient. Actual: revert AlreadyPaid(R1); T.balanceOf(recipientStealth) stays 0; the Pay page preflight (src/lib/payer.ts:296-298) reports 'This link was already paid' and refuses to send. Note: a self-transfer (stealthAddress == attacker) fails the balance-delta check at line 121, which is why a second attacker-controlled address is used. **Proof**: a Foundry test that fails on this code and passes once it is fixed. ```solidity // SPDX-License-Identifier: AGPL-3.0-or-later pragma solidity 0.8.23; import "forge-std/Test.sol"; import {QuietPayRouterV2} from "contracts/src/QuietPayRouterV2.sol"; import {QuietPayTestUSDG} from "contracts/src/QuietPayTestUSDG.sol"; import {ERC5564Announcer} from "contracts/src/ERC5564Announcer.sol"; /// A one-time link (fixed amount 7 tUSDG) is burned by a third party who holds the link: /// they call payOnce with the link's request ID, their own address as the "one-time address" /// and amount 1 (one base unit, 0.000001 tUSDG). `paidOnce[requestId]` is set, so the real /// payer's payOnce for the full 7 tUSDG reverts with AlreadyPaid and the recipient never gets /// paid. Fails on the current code; passes once a payment of less than the link's amount to a /// self-chosen address can no longer consume the request ID (or payOnce otherwise lets the /// genuine payment through). contract RouterV2OneTimeLinkBurnTest is Test { QuietPayRouterV2 router; QuietPayTestUSDG token; ERC5564Announcer announcer; address payer = address(0xA11CE); address attacker = address(0xBAD); /// A second address the attacker controls (a self-transfer would give a zero balance delta). address attackerSink = address(0xBAD2); address recipientStealth = address(0x5EA1); bytes16 requestId = 0x0102030405060708090a0b0c0d0e0f10; uint256 linkAmount = 7_000_000; // 7 tUSDG function setUp() public { token = new QuietPayTestUSDG(); announcer = new ERC5564Announcer(); router = new QuietPayRouterV2(address(token), address(announcer)); vm.prank(payer); token.faucet(linkAmount); vm.prank(attacker); token.faucet(1); } function metadata(uint256 amount) internal view returns (bytes memory) { // view tag (1) | transfer selector (4) | token (20) | amount (32) | "QP" magic+version (3) | request ID (16) return abi.encodePacked(bytes1(0x42), bytes4(0xa9059cbb), address(token), amount, bytes3(0x515001), requestId); } function ephemeralKey() internal pure returns (bytes memory) { return abi.encodePacked(bytes1(0x02), bytes32(uint256(1))); } function test_linkHolderBurnsOneTimeLinkForOneBaseUnit() public { // Attacker "pays" the link's request ID to themself with one base unit. vm.startPrank(attacker); token.approve(address(router), 1); router.payOnce(requestId, attackerSink, 1, ephemeralKey(), metadata(1)); vm.stopPrank(); assertEq(token.balanceOf(attackerSink), 1, "attacker kept their unit"); // The genuine payer now pays the full invoice to the recipient's one-time address. vm.startPrank(payer); token.approve(address(router), linkAmount); router.payOnce(requestId, recipientStealth, linkAmount, ephemeralKey(), metadata(linkAmount)); vm.stopPrank(); assertEq(token.balanceOf(recipientStealth), linkAmount, "recipient must receive the invoice amount"); } } ``` ### 2. Medium: Immediate per-deposit approval roots plus the pool's latest-root-only rule let one depositor keep every private withdrawal proof stale `src/app/api/pool/approvals/route.ts:48` ``` if (last && now - Number(last.timestamp) < 10) { ``` Reported by the economics specialist; reproduced by tracing the three pieces. (1) PrivacyPool.validWithdrawal (contracts/vendor/privacy-pools/src/contracts/PrivacyPool.sol:59) accepts a withdrawal only when _proof.ASPRoot() == ENTRYPOINT.latestRoot(); the Entrypoint keeps no root history for the approval set (Entrypoint.sol:279-282 returns only the last element). (2) The approver endpoint is public, rate-limited to 30 calls per minute per client, and its only pacing is the 10-second gap at route.ts:48; refreshApprovals (src/lib/pool/relayer.ts:220-275) sends updateRoot whenever the approved label set has grown, and with POOL_APPROVAL_DELAY_SECONDS at its default 0 (src/lib/pool/server.ts:79) every new deposit grows it at once. (3) The browser builds the withdrawal proof against info.aspRoot read at prepare time (src/lib/pool/actions.ts:104-110, 193), then POSTs to the relayer, which simulates Entrypoint.relay and maps IncorrectASPRoot to 'The approval list changed' (relayer.ts:158-161); PoolFlow.tsx:315-348 re-proves exactly once and scheduled deliveries (src/lib/pool/deliver.ts:79) retry once. So one actor who makes a minimum deposit (1 tUSDG, free from the faucet) and calls POST /api/pool/approvals every 10 seconds forces a new latestRoot at the throttle rate. Any honest withdrawal whose fetch-events, prove (Groth16 in the browser, several seconds) and relay cycle does not fit inside a single 10-second root window is rejected, and the one automatic retry hits the next root. Funds are not lost (ragequit does not depend on the ASP root), but the private exit the pool exists for is unavailable for everyone while the loop runs. Attacker cost per 10 seconds: one deposit's gas; tUSDG is free and the deposits are recoverable by ragequit. Side effect: the approver signs each updateRoot with the relayer key (route.ts:61-67), so the attacker also spends the relayer's ETH at one transaction per 10 seconds. Each piece is correct on its own; the defect is the seam between the contract's latest-root rule and the approver's immediate-publish policy. Fix in the approver, not the vendored contracts: publish roots on a fixed cadence that comfortably exceeds proving plus relay time (for example at most one root per few minutes, or only when the pending set has been stable for N seconds), expose the next-root time in GET /api/relay so clients can start proving right after a root lands, and have the relayer hold a proof until the next window rather than reject it. **Reproduction** State: pool registered (minimumDeposit 1_000_000, vettingFeeBPS 0, approval delay 0), approver running with the relayer key, victim V holds an approved note. Deterministic variant in the tests/e2e-pool.test.ts setup: V calls prepareWithdrawal (proof carries aspRoot = R_k, the current latestRoot); before relayWithdrawal runs, attacker A does T.faucet(1_000_000); T.approve(entrypoint, 1_000_000); Entrypoint.deposit(T, 1_000_000, freshPrecommitment); then refreshApprovals(...) is invoked (the POST /api/pool/approvals path once 10 s have passed since the last RootUpdated). Observed by code: refreshApprovals computes a root over one more label, which differs from info.aspRoot (relayer.ts:258-259), and sends updateRoot, so latestRoot becomes R_(k+1). V's relayWithdrawal: the simulateContract at relayer.ts:158 reverts IncorrectASPRoot (PrivacyPool.sol:59) and the relayer answers 422 'The approval list changed. Generate the proof again.' Expected: a proof built against the approval set that contains V's deposit is relayed. Actual: rejected; repeating A's step every 10 s makes the Pool page's single retry (PoolFlow.tsx:335-346) and deliver.ts's single retry fail the same way, and no private withdrawal lands while the loop runs. ### 3. Low: QuietPayRouter v1 announces caller-chosen metadata and treats a codeless token address as a successful transfer `contracts/src/QuietPayRouter.sol:55` ``` if (!ok || (ret.length != 0 && !abi.decode(ret, (bool)))) revert TokenTransferFailed(); ``` Merged from three specialist reports (economics: metadata not verified; permissions and flow: codeless token) that share one root cause: the v1 router takes token and announcer from the caller (lines 37-44), checks only that they are non-zero (line 48), and never compares the announcement with the transfer. Two consequences. (a) A low-level call to an address with no code returns ok == true with empty return data, so line 55 does not revert; execution forwards the stipend and calls announcer.announce (line 62), producing an Announcement whose indexed caller is the router with no token transfer behind it, contradicting the NatSpec promise at lines 14-20 that the announcement and the payment are atomic. (b) Even with the real token, the metadata's token and amount fields and the ephemeral key are not checked, and the recipient's balance delta is not verified, so an announcement can describe a different amount or token than was moved. v2 fixes both (constructor NotAContract check at QuietPayRouterV2.sol:64, _checkTransferMetadata at lines 102-106, balance delta at line 121). v1 is still accepted by the app: its code hash is in KNOWN_ROUTER_CODE_HASHES (src/config/contracts.ts:18-20) and verifyRouter returns version 1 for it (src/lib/chain.ts:143), so any network configured with a v1 router is exposed. The testnet (.env.production) uses the v2 deployment. Impact is limited to announcement pollution that looks router-originated: the app only shows a payment as claimable after it finds real Transfer logs, and a payer who sends less than announced only misleads themselves. Fix: drop the v1 hash from KNOWN_ROUTER_CODE_HASHES so the app never routes through v1, or, if v1 must stay deployable, add `if (token.code.length == 0) revert TokenTransferFailed();` and the v2 metadata and balance-delta checks. **Reproduction** Deploy QuietPayRouter (v1) R1 and ERC5564Announcer A. (a) Call R1.pay{value: 0}(0x000000000000000000000000000000000000dEaD, A, S, 5_000_000, 0x02 || 32 bytes, 57 bytes of metadata) from any account with no token balance or approval. Expected: revert TokenTransferFailed. Actual: `token.call` to the codeless address returns (true, empty), line 55 passes, and A emits Announcement(1, S, R1, key, metadata) with no Transfer event anywhere. (b) With tUSDG T: payer T.faucet(7_000_000); T.approve(R1, 1); R1.pay(T, A, S, 1, 0x1234, metadata with uint256(7_000_000) in bytes 25..56). Expected by the v2 rule: revert BadMetadata / BadEphemeralKey. Actual: success; Transfer of 1 base unit to S and an Announcement whose metadata says 7_000_000 and whose ephemeral key is 2 bytes. Deploying QuietPayRouterV2 with the codeless address reverts NotAContract and the mismatched metadata reverts BadMetadata, confirming the gap is v1-only. ### 4. Low: Relayer's fee-covers-gas rule is a no-op against the free faucet token, so its ETH can be drained in bounded amounts `src/lib/pool/relayer.ts:62` ``` const feeWei = (((p.amount * p.feeBPS) / 10_000n) * WEI) / p.tokenUnitsPerEth; ``` Reported by the economics specialist; reproduced by code trace. relayEconomics values the token fee at the configured RELAYER_ETH_PRICE (default 3000 tokens per ETH, src/lib/pool/server.ts:51-58, 80) and allows the relay plus a gas drop of up to RELAYER_GAS_DROP_ETH (default 0.0002 ETH, server.ts:71) when the fee at that price covers them. The pool's token is QuietPayTestUSDG, which anyone mints at up to 10,000 tUSDG per faucet call (contracts/src/QuietPayTestUSDG.sol:29-34), so the 'fee' that is meant to pay the relayer is free to produce and the comment on RelayerPolicy.tokenUnitsPerEth ('someone could make many tiny withdrawals to addresses they control and collect the relayer's ETH') describes exactly what remains possible. Per accepted withdrawal the relayer spends the relay gas (RELAY_GAS_QUOTE 750k with 20% headroom, relayer.ts:164-182) plus the gas drop to any recipient whose balance is below the drop (relayer.ts:168-176, 190-193), in exchange for worthless tokens; withdrawing to a fresh address each time fires the drop every time. The only bound is the rate limit of 30 relays per 10 minutes per client id, which is a salted hash of the forwarded IP (src/app/api/relay/route.ts:15, 64; src/lib/server/guard.ts:132-145), so a few IPs empty a modestly funded hot wallet, after which GET /api/relay reports available:false (route.ts:48) and every user's private withdrawals stop. The docs already list a live price and a balance monitor as mainnet work (docs/shielded-pool.md:203-205); the testnet-specific gap is that the economic guard cannot work while the pool token is mintable for free. Fix: on the mock-token network require a minimum deposit age or a per-recipient drop cooldown, cap the gas drop per recipient and per day, or disable the drop and keep a balance alarm. **Reproduction** Attacker with a little testnet ETH: T.faucet(10_000_000_000); deposit 100 tUSDG notes through Entrypoint.deposit; after approval, for each note run prepareWithdrawal(amount 100_000_000, recipient = fresh empty address, feeBPS 50) and POST the result to /api/relay. Expected per the tokenUnitsPerEth comment: the fee pays for the relay and the top-up, so the relayer's ETH does not fall in net terms. Actual per relay: relayEconomics sees feeWei = (100_000_000 * 50 / 10_000) * 1e18 / 3000e6 = 1.67e14 wei (0.000167 ETH); at a 0.1 gwei gas price relayCost = 900_000 * 1e8 = 9e13 wei, so surplus = 7.7e13 wei, ok == true and drop = min(surplus - 21_000 * 1e8, 0.0002 ETH) = about 0.000075 ETH; the relayer pays the relay gas and sends the drop to the fresh address, receiving 0.5 tUSDG that cost nothing to mint. Thirty such relays per 10-minute window per client id, repeated from several IPs, drive the relayer balance below gasDrop * 2, at which point /api/relay returns available:false. ### 5. Low: QuietPayRouterV2 accepts its own address as the stealth address and permanently locks the tokens it receives `contracts/src/QuietPayRouterV2.sol:109` ``` if (stealthAddress == address(0)) revert ZeroAddress(); ``` Reported by the math specialist; reproduced by code trace. _pay rejects only address(0) as the recipient. With stealthAddress == address(this), transferFrom(msg.sender, router, amount) succeeds for any standard token, the balance-delta check at line 121 passes because the router's own balance rises by exactly amount, the stipend branch is skipped when msg.value == 0, and the announcement is emitted. The contract has no function that moves tokens out (only pay and payOnce exist, and tests/e2e-router-v2.test.ts:296-300 asserts that), so the tokens are frozen forever, contradicting the header's 'it never holds funds between calls' (line 25). The same holds for stealthAddress == token. The app always derives a fresh stealth address, so only a caller who crafts the call directly loses funds, which keeps this low. Fix: `if (stealthAddress == address(0) || stealthAddress == address(this) || stealthAddress == token) revert ZeroAddress();` (or a dedicated BadRecipient error). **Reproduction** Deploy QuietPayTestUSDG T, ERC5564Announcer A, QuietPayRouterV2 R(T, A). Payer: T.faucet(1_000_000); T.approve(R, 1_000_000); R.pay{value: 0}(address(R), 1_000_000, 0x02 || 32 bytes, meta57) with meta57 = 0x42 || 0xa9059cbb || bytes20(T) || uint256(1_000_000). Expected: revert (the router must never hold funds). Actual: success, Announcement emitted with stealthAddress == R, T.balanceOf(R) == 1_000_000, and no code path can ever reduce it. With msg.value > 0 the same call reverts StipendTransferFailed because the router has no receive function, which shows the self-address case was only ever blocked by accident on the ETH leg. ### 6. Info: Trust assumption: the Entrypoint owner can set a vetting fee up to 99.99% that applies to deposits already in flight; the owner is the CI deployer key `contracts/vendor/privacy-pools/src/contracts/Entrypoint.sol:396` ``` if (_vettingFeeBPS >= 10_000 || _maxRelayFeeBPS >= 10_000) revert InvalidFeeBPS(); ``` Reported by the permissions specialist; confirmed as a documented privileged power, not a bypass. updatePoolConfiguration (lines 239-253, onlyRole(_OWNER_ROLE)) accepts any vetting fee below 10_000 bps with no delay; _handleDeposit (line 332) reads the fee in force when the deposit executes, and neither deposit overload (lines 111-126) takes a maximum-fee or minimum-received argument. A deposit sent while the fee is 0 bps (the testnet value, scripts/deploy-testnet.ts:142) is charged whatever fee is in force when it is mined, and the retained amount is sweepable through withdrawFees (lines 264-272). This is unchanged upstream 0xbow code and intentional. The actor matters: scripts/deploy-testnet.ts:139 makes the owner the GitHub Actions deployer account, and docs/shielded-pool.md:165-169 says only that the owner can 'change fees within limits'. Record precisely that the owner key can take up to 99.99% of any deposit that lands after a fee change. Mitigation within the agreed design: owner behind a multisig and timelock before any mainnet pool (already listed at docs/shielded-pool.md:202), and have the client read the fee in the same block as the deposit and refuse when it changed, or pass an expected fee if the Entrypoint is ever extended. **Reproduction** State: pool registered with vettingFeeBPS = 0. User U submits deposit(tUSDG, 100_000_000, precommitment) expecting 100 tUSDG credited. Owner O submits updatePoolConfiguration(tUSDG, 1_000_000, 9_999, 100) and it is mined first. U's deposit executes with _config.vettingFeeBPS = 9_999: _deductFee gives 100_000_000 - (100_000_000 * 9_999 / 10_000) = 10_000 base units (0.01 tUSDG) into the pool commitment, 99_990_000 stay in the Entrypoint. O calls withdrawFees(tUSDG, O) and receives 99.99 tUSDG. Expected by the user: the 0% fee shown when the deposit was built. Actual: 99.99% taken, with no onchain parameter letting the user bound the fee. Precondition: owner key acts maliciously or is compromised. ### 7. Info: Trust assumption: the UUPS owner can upgrade the Entrypoint and keep relayed withdrawals in flight, which the security notes say the owner cannot do `contracts/vendor/privacy-pools/src/contracts/Entrypoint.sol:308` ``` function _authorizeUpgrade(address) internal override onlyRole(_OWNER_ROLE) {} ``` Reported by the permissions specialist; confirmed as a documented privileged power, not a bypass. _authorizeUpgrade lets _OWNER_ROLE replace the implementation at any time with no timelock. In the relay path PrivacyPool.withdraw pushes the withdrawn tokens to _withdrawal.processooor (PrivacyPool.sol:124), which for every relayed withdrawal must be the Entrypoint (Entrypoint.sol:141), and only then does Entrypoint.relay forward them to _data.recipient and the fee recipient (lines 167-169). The proof binds the recipient inside _withdrawal.data, but the pool pays the Entrypoint and trusts its code to forward. A replacement implementation whose relay omits the two _transfer calls keeps every relayed withdrawal that lands after the upgrade, and withdrawFees (line 264) then sweeps it. Direct withdrawals (processooor = the user) and ragequit do not pass through the Entrypoint, so pool principal at rest is safe, which is what SECURITY.md:209-210 and docs/shielded-pool.md:166-168 describe ('The owner cannot take pool funds'). The gap is funds in transit through the Entrypoint, that is every withdrawal made with the QuietPay relayer, and the owner is currently the CI deployer key (scripts/deploy-testnet.ts:139). Upstream design; record the power precisely in SECURITY.md and docs/shielded-pool.md, and before mainnet put the owner behind a multisig and timelock so users can exit or withdraw directly before an upgrade takes effect. **Reproduction** State: Entrypoint proxy E owned by O, pool P registered for tUSDG, user U has an approved commitment of 50 tUSDG. O deploys an implementation whose relay(...) calls _pool.withdraw(_withdrawal, _proof) and returns without the two _transfer calls, then calls E.upgradeToAndCall(newImpl, ""). U (or the relayer for U) submits relay(withdrawal{processooor: E, data: {recipient: U2, feeRecipient: relayer, relayFeeBPS: 50}}, proof, scope). Actual: P pushes 50 tUSDG to E (PrivacyPool.sol:124), the new relay keeps them, E.withdrawFees(tUSDG, O) sweeps 50 tUSDG to O. Expected per SECURITY.md:209: the owner cannot obtain user withdrawals. Precondition: owner key acts maliciously or is compromised; no unprivileged actor can trigger this. --- Judge's submission `6ee0066c4832f4e5b1e245e273bf04d1464eeb8e3d482dd5a2b3274617432927`, accepted on the IdentityMD network. Acceptance means the report met the job's checks; it is not a guarantee that the code has no other defects.