Repository navigation
fix(security): verify payments locally and stop caching paid responses - #11
Merged
Merged
Conversation
Closes #6 (Attack II, partial) and #4 (Attack III). LOCAL VERIFICATION (#6) gate() forwarded the payload to facilitator.settle() and released the resource on success, doing no checks of its own. That made the facilitator the entire security boundary — and `cfg.facilitator` is a documented, first-class option, so "the hosted one" and "any third party's" are the same code path. Any weakness there was a full bypass with no second line. verifyLocally() now checks the presented payload against the requirements this server built, before the facilitator sees it: payTo is the authorization actually addressed to us amount value >= maxAmountRequired (overpaying is the payer's business) scheme matches the challenge network matches the challenge x402Version checked first, because a payload from another version may use the same field names for different things validity expired, or not-yet-valid beyond a 5s skew allowance shape a signature and an authorization object are present Running it BEFORE settle matters as much as the checks: a mismatched payment is now never submitted at all, rather than settled on-chain for a resource that is then refused. The tests use a facilitator stub that approves everything, so each refusal is demonstrated with the facilitator actively saying yes — a test with an honest facilitator would pass whether or not these checks existed. Address comparison is chain-aware. EVM addresses are hex and case-insensitive, so EIP-55 checksumming means the same address legitimately arrives in different cases and a byte comparison would reject correct payments. Solana addresses are base58, where case is significant and lowercasing would make two DIFFERENT addresses compare equal. Blanket-lowercasing is wrong on one chain or the other, so the shape decides. `skipLocalVerification` exists for a scheme this package cannot interpret. It is named for what it does rather than something reassuring, because setting it restores the behaviour where a facilitator weakness is a full bypass. NOT CLOSED BY THIS. These are field checks, not a signature check and not a resource binding. The `exact` scheme's authorization carries no resource, so a payment minted for resource A still satisfies every field of resource B on the same server at the same price. Cross-resource substitution needs a binding the payer signs over — F1 in @furlpay/x402-guard — and #6 also asks for middleware replay/idempotency, which needs a nonce store this package does not have. Both are called out in the README rather than implied by a passing check. CACHE-CONTROL (#4) Neither adapter set any caching policy, so a gated route behind a CDN or reverse proxy could have its PAID response cached and replayed to later UNPAID clients — one payment, unlimited grants, with the payment layer never involved. Both adapters now stamp `Cache-Control: no-store, private` and `Vary: X-PAYMENT`, on the 402 challenge as well as the 200. A cached challenge is a cached quote, and every payer served it would be answering the same one. withX402 SETS rather than appends: a handler that chose its own cache policy chose it without knowing the response was paid for. TEST FIXTURES Three pre-existing fixtures were `{ x402Version: 1 }` — no scheme, no network, no authorization. They exercised the settle path only because gate() checked nothing and forwarded whatever arrived; no real facilitator could have settled them either. They are now realistic payments. settlement-boundary.test.js needed the same change, and its own anti-vacuity guard is what caught it: the strength sweep stopped reaching a release at all, and the test said so rather than passing empty. VERIFIED npm test 45 tests, 45 pass, 0 fail (28 pre-existing + 17 new) Zero new dependencies.
Ashutosh0x
added a commit
that referenced
this pull request
Sep 25, 2026
Completes #6. The field checks that landed in #11 covered payTo, amount, scheme, network, version and validity; these are the two halves those checks structurally cannot reach. RESOURCE BINDING Cross-resource substitution is invisible to field validation. A payment for /a and a payment for /b, at the same price on the same server, differ in NO field the authorization carries — the `exact` scheme does not name a resource. Every check passes on both, because every field genuinely matches. `bindingSecret` makes the server mint a quote when it issues the 402 and require it back with the payment. The quote is HMAC'd over resource, price, recipient and network, so a payer can echo one but cannot mint one for another route, and a quote stops verifying the moment any of those terms change. It rides in `PaymentRequirements.extra` outbound and `PaymentPayload.extra` inbound, both of which the spec already carries — no wire-format change. The signature is checked before any claim is read out of the body: a forged quote's contents are attacker-controlled, and comparing them first would be deciding on unauthenticated data. Comparison is constant-time and length-guarded, because timingSafeEqual throws on a length mismatch and that throw is itself a timing signal. Opt-in, because requiring payers to echo a quote is a protocol change for them — not because it is optional in any security sense. SINGLE-USE CLAIMS Replay resistance was entirely the facilitator's; the paper observed 248 grants from one payment against a facilitator that did not linearize. `gate()` now claims each payment before settling, so concurrent copies cannot both reach the settler. THE RELEASE RULE IS THE DESIGN: success keep the payment is spent definite failure RELEASE no money moved; locking a payer out of their own unspent authorization is a penalty the protocol does not have not deep enough RELEASE that 402 invites a retry, and holding the claim would make "wait and try again" the one thing the payer cannot do threw / timed out KEEP UNKNOWN IS NOT "DID NOT HAPPEN". The transaction may have landed and the response been lost; releasing re-opens replay for the payment most likely to have actually settled That last branch is the one that is tempting to get wrong — releasing on a timeout feels tidy and is the dangerous option. Claim keys are network-scoped (a nonce is only unique within its chain) and quote-scoped when binding is on, so the key follows the SERVER-issued id rather than a nonce the payer chooses and could vary at will. The default store is in-memory and documented as single-instance only: its atomicity is Node being single-threaded, which is a real guarantee in one process and worth nothing across two workers. `claimStore` accepts a shared implementation; `singleUse: false` opts out entirely. VERIFIED npm test 66 tests, 66 pass, 0 fail (45 before, 21 new) Zero new dependencies — binding uses node:crypto.
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 #4. Partially addresses #6.
#6 —
gate()no longer delegates the whole decisiongate()forwarded the payload tofacilitator.settle()and released onsuccess, checking nothing itself. Sincecfg.facilitatoris a documentedoption, "the hosted one" and "any third party's" are the same code path, so any
weakness there was a full bypass with no second line.
verifyLocally()now checks the payload against the requirements this serverbuilt —
payTo, amount (>=, so overpaying is fine),scheme,network,x402Version, the validity window, and basic shape.Two things worth calling out:
than settled on-chain for a resource that is then refused.
is demonstrated with the facilitator actively saying yes. A test with an
honest facilitator would pass whether or not these checks existed.
Address comparison is chain-aware: EVM addresses are hex and case-insensitive
(EIP-55 checksumming means the same address legitimately arrives in different
cases), while Solana addresses are base58 where case is significant and
lowercasing would make two different addresses compare equal.
What this does not close
These are field checks — not a signature check, and not a resource binding. The
exactscheme's authorization carries no resource, so a payment minted forresource A still satisfies every field of resource B on the same server at the
same price. Cross-resource substitution needs a binding the payer signs over
(F1 in
@furlpay/x402-guard). #6 also asks for middleware-levelreplay/idempotency, which needs a nonce store this package does not have.
Both gaps are documented in the README rather than left implied by a passing
check, so I have left #6 open.
#4 — paid responses are no longer cacheable
Both adapters now stamp
Cache-Control: no-store, privateandVary: X-PAYMENT, on the 402 challenge as well as the 200 — a cached challengeis a cached quote.
withX402sets rather than appends, because a handler thatchose its own cache policy chose it without knowing the response was paid for.
Test fixtures
Three pre-existing fixtures were
{ x402Version: 1 }— no scheme, no network,no authorization. They exercised the settle path only because
gate()forwardedwhatever arrived; no real facilitator could have settled them either. They are
now realistic payments.
settlement-boundary.test.jsneeded the same change, and its own anti-vacuityguard is what caught it — the strength sweep stopped reaching a release at all
and the test said so rather than passing empty.
Verified
Zero new dependencies.