From 8615a9f2b8069bc442b8b3237802570ba516927f Mon Sep 17 00:00:00 2001 From: Ryan Aubrey Date: Tue, 1 Sep 2026 13:54:46 -0400 Subject: [PATCH] fix(tempo): always bind payments to the challenge memo A method-wide explicit memo skipped challenge binding, so one settled transfer could satisfy every equivalent challenge. Require the MPP attribution memo on every path, including when a merchant memo is set. Co-authored-by: Cursor Committed-By-Agent: cursor --- .../mpp/methods/tempo/TempoChargeIntent.java | 15 ++-- .../stripe/mpp/methods/tempo/TempoMethod.java | 11 +-- .../methods/tempo/TempoChargeIntentTest.java | 81 +++++++++++++++---- 3 files changed, 80 insertions(+), 27 deletions(-) diff --git a/src/main/java/com/stripe/mpp/methods/tempo/TempoChargeIntent.java b/src/main/java/com/stripe/mpp/methods/tempo/TempoChargeIntent.java index f5ab16c..3504572 100644 --- a/src/main/java/com/stripe/mpp/methods/tempo/TempoChargeIntent.java +++ b/src/main/java/com/stripe/mpp/methods/tempo/TempoChargeIntent.java @@ -29,11 +29,12 @@ * * *

A qualifying Transfer of the requested token, recipient and amount is not - * enough. Unless the merchant set an explicit memo, the matched logs must include - * a {@code TransferWithMemo} whose memo is bound to this challenge (MPP attribution - * tag, server fingerprint of the challenge realm, and nonce - * {@code keccak256(challengeId)[0..6]}). That is what stops a third party from - * presenting someone else's settled transaction as their own payment. + * enough. The matched logs must include a {@code TransferWithMemo} whose memo + * is bound to this challenge (MPP attribution tag, server fingerprint of the + * challenge realm, and nonce {@code keccak256(challengeId)[0..6]}). A merchant + * memo, when present, is an additional exact-match constraint — it does not + * skip that binding. That is what stops a third party from presenting someone + * else's settled transaction as their own payment. * *

Create the intent once and reuse it so its replay store is shared across requests: * @@ -159,9 +160,7 @@ private Receipt awaitReceipt( "transaction logs contain no Transfer matching the request currency, recipient, and amount" ); } - if (memoFrom(request) == null) { - assertChallengeBoundMemo(matched, credential); - } + assertChallengeBoundMemo(matched, credential); return Receipt.success(txHash, "tempo"); } if (i < maxRetries - 1) { diff --git a/src/main/java/com/stripe/mpp/methods/tempo/TempoMethod.java b/src/main/java/com/stripe/mpp/methods/tempo/TempoMethod.java index ee0a8f9..3eb71ce 100644 --- a/src/main/java/com/stripe/mpp/methods/tempo/TempoMethod.java +++ b/src/main/java/com/stripe/mpp/methods/tempo/TempoMethod.java @@ -104,12 +104,13 @@ public Builder store(Store store) { } /** - * Sets an explicit TIP-20 memo that payments must match. + * Sets an explicit TIP-20 memo that payments must match in addition to + * the challenge-bound MPP attribution memo. * - *

When omitted, clients write an MPP attribution memo bound to the - * challenge and the server requires that binding. An explicit memo is - * matched exactly and is not challenge-bound — the caller must make it - * unique per challenge if hash reuse across challenges should be rejected. + *

Verification always requires an on-chain memo bound to the + * challenge id and realm. A method-wide static value cannot satisfy + * that binding across distinct challenges, so omit this unless the + * advertised memo is itself unique per challenge. */ public Builder memo(String memo) { this.memo = Objects.requireNonNull(memo, "memo"); diff --git a/src/test/java/com/stripe/mpp/methods/tempo/TempoChargeIntentTest.java b/src/test/java/com/stripe/mpp/methods/tempo/TempoChargeIntentTest.java index c61adf9..0056d70 100644 --- a/src/test/java/com/stripe/mpp/methods/tempo/TempoChargeIntentTest.java +++ b/src/test/java/com/stripe/mpp/methods/tempo/TempoChargeIntentTest.java @@ -49,13 +49,17 @@ static Credential txCredential(String rawTx) { } static Credential hashCredential(String txHash) { - return hashCredential(txHash, null); + return hashCredential(txHash, (String) null); } static Credential hashCredential(String txHash, String source) { return new Credential(ECHO, Map.of("type", "hash", "hash", txHash), source); } + static Credential hashCredential(String txHash, ChallengeEcho echo) { + return new Credential(echo, Map.of("type", "hash", "hash", txHash), null); + } + static String didPkh(int chainId, String address) { return "did:pkh:eip155:" + chainId + ":" + address; } @@ -370,12 +374,11 @@ void pushAcceptsChallengeBoundMemoAlongsideAPlainTransfer() { @Test void explicitMemoMustMatchExactly() { - String merchantMemo = "0x" + "ab".repeat(32); Map request = new HashMap<>(REQUEST); - request.put("memo", merchantMemo); + request.put("memo", BOUND_MEMO); Receipt result = intent(new StubRpc(null, - receiptWithMemoLog(TOKEN_CONTRACT, SENDER, RECIPIENT, AMOUNT_ATOMIC, merchantMemo), 0)) + receiptWithMemoLog(TOKEN_CONTRACT, SENDER, RECIPIENT, AMOUNT_ATOMIC, BOUND_MEMO), 0)) .verify(hashCredential("0xpushedtx"), request); assertThat(result.status()).isEqualTo("success"); } @@ -383,31 +386,31 @@ void explicitMemoMustMatchExactly() { @Test void explicitMemoMismatchIsRejected() { String merchantMemo = "0x" + "ab".repeat(32); - String otherMemo = "0x" + "cd".repeat(32); Map request = new HashMap<>(REQUEST); request.put("memo", merchantMemo); assertThatThrownBy(() -> intent(new StubRpc(null, - receiptWithMemoLog(TOKEN_CONTRACT, SENDER, RECIPIENT, AMOUNT_ATOMIC, otherMemo), 0)) + receiptWithMemoLog(TOKEN_CONTRACT, SENDER, RECIPIENT, AMOUNT_ATOMIC, BOUND_MEMO), 0)) .verify(hashCredential("0xpushedtx"), request)) .isInstanceOf(VerificationFailedException.class) .hasMessageContaining("Transfer"); } @Test - void explicitMemoDoesNotRequireChallengeBinding() { + void explicitMemoDoesNotBypassChallengeBinding() { String merchantMemo = "0x" + "ab".repeat(32); Map request = new HashMap<>(REQUEST); request.put("memo", merchantMemo); - Receipt result = intent(new StubRpc(null, + assertThatThrownBy(() -> intent(new StubRpc(null, receiptWithMemoLog(TOKEN_CONTRACT, SENDER, RECIPIENT, AMOUNT_ATOMIC, merchantMemo), 0)) - .verify(hashCredential("0xpushedtx"), request); - assertThat(result.status()).isEqualTo("success"); + .verify(hashCredential("0xpushedtx"), request)) + .isInstanceOf(VerificationFailedException.class) + .hasMessageContaining("memo is not bound to this challenge"); } @Test - void explicitMemoInMethodDetailsIsHonored() { + void explicitMemoInMethodDetailsStillRequiresChallengeBinding() { String merchantMemo = "0x" + "ab".repeat(32); Map request = Map.of( "amount", String.valueOf(AMOUNT_ATOMIC), @@ -416,10 +419,60 @@ void explicitMemoInMethodDetailsIsHonored() { "methodDetails", Map.of("chainId", CHAIN_ID, "memo", merchantMemo) ); - Receipt result = intent(new StubRpc(null, + assertThatThrownBy(() -> intent(new StubRpc(null, receiptWithMemoLog(TOKEN_CONTRACT, SENDER, RECIPIENT, AMOUNT_ATOMIC, merchantMemo), 0)) - .verify(hashCredential("0xpushedtx"), request); - assertThat(result.status()).isEqualTo("success"); + .verify(hashCredential("0xpushedtx"), request)) + .isInstanceOf(VerificationFailedException.class) + .hasMessageContaining("memo is not bound to this challenge"); + } + + @Test + void sharedExplicitMemoCannotSatisfyADifferentChallenge() { + ChallengeEcho echoA = new ChallengeEcho( + "chal-a", "api.example.com", "tempo", "charge", "e30", "2099-01-01T00:00:00Z", null, null + ); + ChallengeEcho echoB = new ChallengeEcho( + "chal-b", "api.example.com", "tempo", "charge", "e30", "2099-01-01T00:00:00Z", null, null + ); + String merchantMemo = "0x" + "ab".repeat(32); + Map request = new HashMap<>(REQUEST); + request.put("memo", merchantMemo); + Map paidForB = receiptWithMemoLog( + TOKEN_CONTRACT, SENDER, RECIPIENT, AMOUNT_ATOMIC, merchantMemo); + + Store store = new MemoryStore(); + assertThatThrownBy(() -> intent(new StubRpc(null, paidForB, 0), store) + .verify(hashCredential("0xvictimtx", echoA), request)) + .isInstanceOf(VerificationFailedException.class) + .hasMessageContaining("memo is not bound to this challenge"); + + assertThatThrownBy(() -> intent(new StubRpc(null, paidForB, 0), store) + .verify(hashCredential("0xvictimtx", echoB), request)) + .isInstanceOf(VerificationFailedException.class) + .hasMessageContaining("memo is not bound to this challenge"); + } + + @Test + void stolenBoundPaymentCannotSatisfyADifferentChallenge() { + ChallengeEcho echoA = new ChallengeEcho( + "chal-a", "api.example.com", "tempo", "charge", "e30", "2099-01-01T00:00:00Z", null, null + ); + ChallengeEcho echoB = new ChallengeEcho( + "chal-b", "api.example.com", "tempo", "charge", "e30", "2099-01-01T00:00:00Z", null, null + ); + String memoForB = Attribution.encode(echoB.realm(), echoB.id()); + Store store = new MemoryStore(); + + assertThatThrownBy(() -> intent(new StubRpc(null, + receiptWithMemoLog(TOKEN_CONTRACT, SENDER, RECIPIENT, AMOUNT_ATOMIC, memoForB), 0), store) + .verify(hashCredential("0xvictimtx", echoA), REQUEST)) + .isInstanceOf(VerificationFailedException.class) + .hasMessageContaining("memo is not bound to this challenge"); + + Receipt result = intent(new StubRpc(null, + receiptWithMemoLog(TOKEN_CONTRACT, SENDER, RECIPIENT, AMOUNT_ATOMIC, memoForB), 0), store) + .verify(hashCredential("0xvictimtx", echoB), REQUEST); + assertThat(result.reference()).isEqualTo("0xvictimtx"); } @Test