fix(stripe): reject idempotent-replayed PaymentIntents in charge verification - #21
Open
ygd58 wants to merge 1 commit into
Open
fix(stripe): reject idempotent-replayed PaymentIntents in charge verification#21ygd58 wants to merge 1 commit into
ygd58 wants to merge 1 commit into
Conversation
|
|
This was referenced Aug 13, 2026
ygd58
added a commit
to ygd58/mpp-java
that referenced
this pull request
Aug 23, 2026
matchTransferLogs accepted any TransferWithMemo (or plain Transfer) log that matched currency/recipient/sender/amount, without ever inspecting the memo content. Canonical mppx and mpp-go both require, when a TransferWithMemo log carries the MPP attribution memo layout, that its challenge nonce is bound to the specific challenge being verified (server fingerprint = keccak256(realm)[0..9], nonce = keccak256(challengeId)[0..6]). This SDK skipped that check entirely, so a transaction whose attribution memo was minted for a *different* challenge (or realm) — but which otherwise matches this challenge's payment terms exactly — was accepted as satisfying this challenge. Adds Attribution.java, ported from wevm/mppx's src/tempo/Attribution.ts (and matching tempoxyz/mpp-go's pkg/tempo/attribution.go, which is why Go was clean in the audit) using BouncyCastle's Keccak.Digest256 — already a main dependency here via TempoRelay.java, so no new dependency needed. Scope, per the discussion on the linked issue: this only tightens TransferWithMemo handling. A memo that doesn't use the MPP attribution layout at all (an application-defined memo, or no memo/plain Transfer) is left untouched, so existing non-memo payment flows keep working unchanged — this closes the replay path without a breaking change to currently-supported plain-Transfer verification. Adds: - AttributionTest: direct coverage of the byte-layout verification (bound/unbound realm, bound/unbound challenge, non-attribution memo, malformed input never throws). - TempoChargeIntentTest: updates the existing transferWithMemoTopicAccepted test to use a properly bound memo (previously used an arbitrary unbound value and only happened to pass because binding wasn't checked at all), adds regression tests for memos bound to a different challenge and a different realm, and a test confirming the plain Transfer (no memo) flow is unaffected. As with stripe#21 and stripe#18, I could not compile/run this against the real com.stripe:stripe-java / bouncycastle dependencies in my sandbox (no Maven Central access) — please double-check compilation before merge. Fixes tempoxyz/mpp-tools#112 (AGR-2026-036)
…fication StripeApi.createAndConfirm discarded the response entirely except for the PaymentIntent's id and status. Stripe signals a replayed idempotent request via the `Idempotent-Replayed` response header, but that header was never inspected, so StripeChargeIntent.verify() treated a replayed PaymentIntent exactly like a fresh one: any `succeeded` status was accepted and a new success Receipt was issued. A credential (challenge + spt) reused against a different challenge therefore triggered a second successful verification and a second resource grant, even though Stripe only charged the customer once. The canonical TypeScript implementation already guards against this (see wevm/mppx GHSA-8mhj-rffc-rcvw, fixed in mppx 0.4.11) by checking this exact header; this port never carried that check over. Read pi.getLastResponse().headers().firstValue("Idempotent-Replayed") after the create call, thread it through StripeApi.Result as a new idempotentReplayed field (existing 2-arg Result(id, status) constructor kept for source compatibility with existing tests), and reject the credential with VerificationFailedException in StripeChargeIntent.verify() before the status check when it's true. Adds a regression test reproducing the issue: a stubbed Result with status=succeeded and idempotentReplayed=true must throw VerificationFailedException rather than return a success Receipt. I could not compile/run this against the real com.stripe:stripe-java dependency in my environment (no Maven Central access), so I verified the getLastResponse()/headers()/firstValue() call chain directly against the stripe-java v25.3.0 source on GitHub (the version pinned in build.gradle) instead of a live build. Flagging this explicitly — please double-check compilation before merge. Fixes tempoxyz/mpp-tools#111 (AGR-2026-035)
ygd58
force-pushed
the
fix/stripe-idempotent-replay-check
branch
from
August 23, 2026 14:34
746de9e to
59c108b
Compare
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.
What
StripeApi.createAndConfirmdiscarded the create response entirely except for the PaymentIntent's id and status. Stripe signals a replayed idempotent request via theIdempotent-Replayedresponse header, but that header was never inspected, soStripeChargeIntent.verify()treated a replayed PaymentIntent exactly like a fresh one: anysucceededstatus was accepted and a new successReceiptwas issued.Impact: a credential (challenge + spt) reused against a different challenge triggers a second successful verification and a second resource grant, even though Stripe only charged the customer once.
The canonical TypeScript implementation already guards against this — see wevm/mppx GHSA-8mhj-rffc-rcvw, fixed in mppx 0.4.11 — by checking this exact header. This port never carried that check over. Flagged by the cross-SDK audit as AGR-2026-035;
/ag fixisn't available for this repo since it's outside Agricola's write scope, so opening this directly.Fix
pi.getLastResponse().headers().firstValue("Idempotent-Replayed")after the create call.StripeApi.Resultas a newidempotentReplayedfield — kept the existing 2-argResult(id, status)constructor (defaulting tofalse) for source compatibility with existing tests.StripeChargeIntent.verify()now rejects the credential withVerificationFailedExceptionwhenidempotentReplayed()is true, before the status check.Testing
Added
idempotentReplayedSucceededChargeIsRejected: a stubbedResultwithstatus=succeeded, idempotentReplayed=truemust throwVerificationFailedExceptionrather than return a successReceipt. Matches the reproduction shape suggested in the linked finding.I could not compile or run this against the real
com.stripe:stripe-javadependency — my sandbox doesn't have Maven Central access. Instead I verified thegetLastResponse()/headers()/firstValue()call chain directly against the stripe-java v25.3.0 source on GitHub (the version pinned inbuild.gradle), which is the same accessor pattern the SDK itself uses internally (idempotencyKey(),requestId()useheaders().firstValue(...)the same way). But I want to be upfront that this hasn't been through an actual./gradlew test— please compile-check before merging, and let me know if anything doesn't match and I'll fix it.Fixes #(the corresponding issue in this repo, if one exists) / tempoxyz/mpp-tools#111