fix(gateway): contain C-03 arbitrary-approval + unverified-fund on escrow chain writes - #274
Merged
Merged
Conversation
…escrow chain writes Two routes let an untrusted caller steer the gateway's own signer (PCC_GATEWAY_PRIVATE_KEY). Auth does not cover them: scope-checker has no /api/escrow/* rule and is open-by-default, and /api/auth/provision is public self-service issuing scopes:["*"]. Both routes are published in the public agent-package.json. 1. POST /api/escrow/chain/:address/approve — took the ERC-20 spender (:address), tokenAddress and amount straight from the request, bounded only by MAX_ESCROW_AMOUNT. An attacker could name their own address as spender and drain the signer's balances via transferFrom. The route now returns 410 Gone unconditionally and accepts no params at all. It stays registered (410, not 404) so existing clients get a clear signal. The legitimate funding allowance is unaffected: it is issued internally by paid-job-flow.createJobFromSession, which approves ONLY a freshly factory-created escrow for the exact fund amount (packages/gateway/src/routes/paid-job-flow.ts:408-414). Verified: nothing in the repo calls the HTTP route. BREAKING (expected and accepted per the audit remediation note): clients can no longer call approve directly. The published tool `approve_token` in apps/dashboard/public/agent-package.json will now receive 410. Left for the owner to decide, along with a release trailer if a major bump is wanted. 2. POST /api/escrow/chain/:address/fund — validated only isAddress(), then forwarded the caller-supplied address into fundEscrowActivity with no DB lookup and no allowlist, so the signer could be pointed at any contract. The address must now resolve to an escrow row this gateway created before any on-chain action. Unknown -> 404 escrow_not_found (an existence question about a well-formed path resource; 400 would mis-describe it as malformed). Registry unreadable -> 503, failing closed. Idempotency-key behaviour and the activity wrapper are unchanged. The provenance lookup reuses the existing findByContractAddress query that previously served only as a V2/V3 version hint, lifted into findEscrowRow so there is one query shape for "do we know this escrow?". It retries the checksummed and lowercased forms because rows are written checksummed while callers routinely send lowercase. Follow-ups the owner must decide (NOT done here): rotate PCC_GATEWAY_PRIVATE_KEY, deploy, add an /api/escrow/* scope rule, and revisit the public self-service provision endpoint. Refs: ai/research/gateway-escrow-approve-fund-verification.md Agent: implementer-c03
11 cases over the two contained routes. The load-bearing assertions are the negative ones: the settlement facade's chain-write methods are spied, and both rejection paths must show zero calls, so a rejection provably never reaches the signer/RPC layer. /approve: 410 with no body, 410 for the exact exploit shape (attacker spender + real Base USDC token + max amount), 410 across every param permutation, 410 for a malformed address (the route validates nothing because it accepts nothing), and 410 rather than 404 so the endpoint stays a clear signal. /fund: 404 for an address with no escrow row and for an attacker-chosen address, both with no chain call; 400 still for a malformed address; 200 for a seeded escrow row; 200 for a lowercase request against a checksum-stored row (guards the casing normalisation from turning into a false negative); and the idempotency-key header still passes through. No network, no wallet, no chain: in-memory SQLite plus a spied facade. Agent: implementer-c03
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
1 participant
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.
Closes the confirmed-exploitable C-03 vector on the live gateway. Ports the owner's own unmerged fix (
47f0a153, Jul 13, stranded onfeat/onramp-user-wallets) and additionally closes/fund, which that fix did not touch.The vulnerability (verified end-to-end)
POST /api/escrow/chain/:address/approvemade the gateway's own signer (PCC_GATEWAY_PRIVATE_KEY) issue an ERC-20approve()where spender, token and amount all came from the request, bounded only byMAX_ESCROW_AMOUNT(1,000,000). Auth existed butscope-checkerhas no/api/escrow/*rule (open-by-default), and/api/auth/provisionis public self-service issuingscopes:["*"]. An attacker names their own address as spender and drains viatransferFrom.It was also advertised:
agent-package.json:707-732published it as toolapprove_token, andfund_escrow's description told agents to "use approve_token first".Blast radius was capped by the
base-sepolia+ MockUSDC config, not by any control in the code — andtokenAddresswas caller-supplied, so real USDC could be named directly.Changes
/approve→ unconditional 410endpoint_removed, accepts no params, stays registered. Rationale comment ported verbatim from the original fix (it is the audit trail)./fund→ provenance gate. Unknown address → 404escrow_not_found(existence question about a well-formed resource, not a malformed request). Registry unreadable → 503, failing closed rather than treating unverified provenance as acceptable. Reuses the existingfindByContractAddresslookup (no new query), retrying checksummed/lowercased forms — rows are written checksummed while callers routinely send lowercase, so an exact-match-only gate would have falsely rejected legitimate funding.Verification
@pcc/gatewaybuild: 31/31 tasks, exit 0escrow-c03-containment.test.ts: 11/11 pass. Load-bearing assertions are negative — the settlement facade'sfundEscrow/approveTokenare spied and both rejection paths assert zero calls, proving a rejection never reaches the signer/RPC layer.it.skip)Forensics
Swept 2,400,000 blocks of Base Sepolia (blocks 42,529,419–44,929,418, ~8 weeks, 0 RPC errors) for
Approvalevents with the gateway signer as owner: 0 found. The signer has sent 334 txs in its entire lifetime and holds only gas. No evidence the endpoint was ever exercised — by an attacker or legitimately (consistent withpaid-job-flowissuing allowances internally). Coverage is bounded; older blocks were not swept.Breaking change (expected, per the original remediation note)
Clients can no longer call
/approve.paid-job-flow.createJobFromSessionis unaffected — it issues its allowance internally (paid-job-flow.ts:408-414), approving only the escrow it just created for the exact fund amount. Verified: no HTTP call to/approveexists in that path.Follow-ups NOT in this PR (owner decisions): rotate
PCC_GATEWAY_PRIVATE_KEY; update/remove theapprove_tokentool in both agent packages; add a/api/escrow/*scope rule; decide whether the removal warrants aBREAKING CHANGE:release-please trailer (omitted here so no major bump fires without your call).🤖 Generated with Claude Code
https://claude.ai/code/session_01HXosKUHrbuWgRS9igRGUgH