From 41db09d9e95d0d5046284715519ba30387841c4e Mon Sep 17 00:00:00 2001 From: Marcelo Salloum Date: Wed, 15 Apr 2026 15:49:27 -0700 Subject: [PATCH 1/4] fix push-mode DoS via semaphore exhaustion --- sdk/src/charge/Methods.test.ts | 23 ++++++++- sdk/src/charge/Methods.ts | 2 +- sdk/src/charge/server/Charge.test.ts | 70 ++++++++++++++++++++++++++++ sdk/src/charge/server/Charge.ts | 24 +++++++--- 4 files changed, 110 insertions(+), 9 deletions(-) diff --git a/sdk/src/charge/Methods.test.ts b/sdk/src/charge/Methods.test.ts index f24b1cf..5846a3a 100644 --- a/sdk/src/charge/Methods.test.ts +++ b/sdk/src/charge/Methods.test.ts @@ -145,16 +145,35 @@ describe('Methods.charge', () => { }) it('credential payload accepts hash type (push)', () => { + const hash = 'a'.repeat(64) const result = Methods.charge.schema.credential.payload.parse({ type: 'hash', - hash: 'abc123', + hash, }) expect(result.type).toBe('hash') if (result.type === 'hash') { - expect(result.hash).toBe('abc123') + expect(result.hash).toBe(hash) } }) + it('credential payload rejects invalid hash format', () => { + expect(() => + Methods.charge.schema.credential.payload.parse({ + type: 'hash', + hash: 'abc123', + }), + ).toThrow() + }) + + it('credential payload rejects hash that is not hex', () => { + expect(() => + Methods.charge.schema.credential.payload.parse({ + type: 'hash', + hash: 'zzzz' + '0'.repeat(60), + }), + ).toThrow() + }) + it('credential payload accepts transaction type (pull)', () => { const result = Methods.charge.schema.credential.payload.parse({ type: 'transaction', diff --git a/sdk/src/charge/Methods.ts b/sdk/src/charge/Methods.ts index b97689d..6393375 100644 --- a/sdk/src/charge/Methods.ts +++ b/sdk/src/charge/Methods.ts @@ -21,7 +21,7 @@ export const charge = Method.from({ credential: { payload: z.discriminatedUnion('type', [ /** Push mode: client broadcasts and sends the tx hash. */ - z.object({ hash: z.string(), type: z.literal('hash') }), + z.object({ hash: z.string().check(z.regex(/^[0-9a-f]{64}$/i)), type: z.literal('hash') }), /** Pull mode: client sends signed XDR as `payload.transaction`, server broadcasts. */ z.object({ transaction: z.string(), type: z.literal('transaction') }), ]), diff --git a/sdk/src/charge/server/Charge.test.ts b/sdk/src/charge/server/Charge.test.ts index 88e2fd9..a3e4ac7 100644 --- a/sdk/src/charge/server/Charge.test.ts +++ b/sdk/src/charge/server/Charge.test.ts @@ -711,6 +711,76 @@ describe('charge hash format validation', () => { }) }) +describe('charge push-mode: single lookup (no polling)', () => { + it('rejects when transaction is NOT_FOUND on-chain', async () => { + mockGetTransaction.mockResolvedValueOnce({ status: 'NOT_FOUND' }) + + const method = charge({ + recipient: RECIPIENT, + currency: USDC_SAC_TESTNET, + store: Store.memory(), + }) + const cred = makeHashCredential({ + hash: 'a'.repeat(64), + source: `did:pkh:stellar:testnet:${Keypair.random().publicKey()}`, + }) + + await expect( + method.verify({ credential: cred as any, request: cred.challenge.request }), + ).rejects.toThrow('Transaction not found on-chain') + }) + + it('rejects when transaction FAILED on-chain', async () => { + mockGetTransaction.mockResolvedValueOnce({ status: 'FAILED' }) + + const method = charge({ + recipient: RECIPIENT, + currency: USDC_SAC_TESTNET, + store: Store.memory(), + }) + const cred = makeHashCredential({ + hash: 'b'.repeat(64), + source: `did:pkh:stellar:testnet:${Keypair.random().publicKey()}`, + }) + + await expect( + method.verify({ credential: cred as any, request: cred.challenge.request }), + ).rejects.toThrow('Transaction failed on-chain') + }) + + it('does not hold a semaphore slot for push-mode lookups', async () => { + // Fake hashes should be rejected instantly without consuming semaphore + // slots — this is the core fix for the DoS vector. + mockGetTransaction.mockResolvedValue({ status: 'NOT_FOUND' }) + + const method = charge({ + recipient: RECIPIENT, + currency: USDC_SAC_TESTNET, + store: Store.memory(), + pollMaxConcurrent: 1, // only 1 semaphore slot + }) + + // Fire 5 concurrent requests with different fake hashes — all must + // reject promptly instead of queueing behind a semaphore. + const start = Date.now() + const results = await Promise.allSettled( + Array.from({ length: 5 }, (_, i) => { + const cred = makeHashCredential({ + hash: `${i}`.repeat(64).slice(0, 64), + source: `did:pkh:stellar:testnet:${Keypair.random().publicKey()}`, + }) + return method.verify({ credential: cred as any, request: cred.challenge.request }) + }), + ) + const elapsed = Date.now() - start + + // All should be rejected (NOT_FOUND) + expect(results.every((r) => r.status === 'rejected')).toBe(true) + // Should complete quickly — no 20s poll timeout per request + expect(elapsed).toBeLessThan(2000) + }) +}) + describe('charge DoS prevention: no global serial lock', () => { it('processes concurrent verify calls in parallel, not serially', async () => { // Regression test: verifyLock was removed to prevent head-of-line blocking. diff --git a/sdk/src/charge/server/Charge.ts b/sdk/src/charge/server/Charge.ts index 2b68b61..91bb97c 100644 --- a/sdk/src/charge/server/Charge.ts +++ b/sdk/src/charge/server/Charge.ts @@ -190,12 +190,24 @@ export function charge(parameters: charge.Parameters) { } releaseClaim(store, hashKey) - const txResult = await pollTransaction(rpcServer, hash, { - maxAttempts: pollMaxAttempts, - delayMs: pollDelayMs, - timeoutMs: pollTimeoutMs, - semaphore: pollSemaphore, - }) + // Push mode requires the transaction to be confirmed on-chain + // before the client submits the hash. A single lookup replaces the + // unbounded poll loop, eliminating the semaphore-exhaustion DoS + // vector (an attacker can no longer hold slots with fake hashes). + const result = await rpcServer.getTransaction(hash) + + if (result.status === 'FAILED') { + throw new PaymentVerificationError(`${LOG_PREFIX} Transaction failed on-chain.`, { hash }) + } + + if (result.status !== 'SUCCESS') { + throw new PaymentVerificationError( + `${LOG_PREFIX} Transaction not found on-chain. Push mode requires the transaction to be confirmed before submitting the hash.`, + { hash, status: result.status }, + ) + } + + const txResult = result as rpc.Api.GetSuccessfulTransactionResponse // Extract the payer's public key from the credential DID to verify // the on-chain transfer's `from` address matches the credential's From 191fb5a5215167001ced8d1abf10ac119cdcdd92 Mon Sep 17 00:00:00 2001 From: Marcelo Salloum Date: Wed, 15 Apr 2026 15:58:17 -0700 Subject: [PATCH 2/4] address PR review: trim comment, include resultXdr --- sdk/src/charge/server/Charge.test.ts | 47 ++++++++++++++++++++++++++++ sdk/src/charge/server/Charge.ts | 9 +++--- 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/sdk/src/charge/server/Charge.test.ts b/sdk/src/charge/server/Charge.test.ts index a3e4ac7..db1080e 100644 --- a/sdk/src/charge/server/Charge.test.ts +++ b/sdk/src/charge/server/Charge.test.ts @@ -748,6 +748,53 @@ describe('charge push-mode: single lookup (no polling)', () => { ).rejects.toThrow('Transaction failed on-chain') }) + it('includes resultXdr in error details when transaction FAILED', async () => { + const fakeResultXdr = 'AAAAAAAAAGT/////AAAAAQAAAAAAAAAB////+wAAAAA=' + mockGetTransaction.mockResolvedValueOnce({ status: 'FAILED', resultXdr: fakeResultXdr }) + + const method = charge({ + recipient: RECIPIENT, + currency: USDC_SAC_TESTNET, + store: Store.memory(), + }) + const cred = makeHashCredential({ + hash: 'c'.repeat(64), + source: `did:pkh:stellar:testnet:${Keypair.random().publicKey()}`, + }) + + try { + await method.verify({ credential: cred as any, request: cred.challenge.request }) + expect.unreachable('should have thrown') + } catch (err: any) { + expect(err.message).toMatch('Transaction failed on-chain') + expect(err.details.resultXdr).toBe(fakeResultXdr) + expect(err.details.hash).toBe('c'.repeat(64)) + } + }) + + it('omits resultXdr from error details when not present in FAILED response', async () => { + mockGetTransaction.mockResolvedValueOnce({ status: 'FAILED' }) + + const method = charge({ + recipient: RECIPIENT, + currency: USDC_SAC_TESTNET, + store: Store.memory(), + }) + const cred = makeHashCredential({ + hash: 'd'.repeat(64), + source: `did:pkh:stellar:testnet:${Keypair.random().publicKey()}`, + }) + + try { + await method.verify({ credential: cred as any, request: cred.challenge.request }) + expect.unreachable('should have thrown') + } catch (err: any) { + expect(err.message).toMatch('Transaction failed on-chain') + expect(err.details.resultXdr).toBeUndefined() + expect(err.details.hash).toBe('d'.repeat(64)) + } + }) + it('does not hold a semaphore slot for push-mode lookups', async () => { // Fake hashes should be rejected instantly without consuming semaphore // slots — this is the core fix for the DoS vector. diff --git a/sdk/src/charge/server/Charge.ts b/sdk/src/charge/server/Charge.ts index 91bb97c..b733336 100644 --- a/sdk/src/charge/server/Charge.ts +++ b/sdk/src/charge/server/Charge.ts @@ -191,13 +191,14 @@ export function charge(parameters: charge.Parameters) { releaseClaim(store, hashKey) // Push mode requires the transaction to be confirmed on-chain - // before the client submits the hash. A single lookup replaces the - // unbounded poll loop, eliminating the semaphore-exhaustion DoS - // vector (an attacker can no longer hold slots with fake hashes). + // before the client submits the hash. const result = await rpcServer.getTransaction(hash) if (result.status === 'FAILED') { - throw new PaymentVerificationError(`${LOG_PREFIX} Transaction failed on-chain.`, { hash }) + throw new PaymentVerificationError(`${LOG_PREFIX} Transaction failed on-chain.`, { + hash, + ...(result.resultXdr ? { resultXdr: result.resultXdr } : {}), + }) } if (result.status !== 'SUCCESS') { From fba6e1b4c02f48a7842f1fb3bae84000e4187405 Mon Sep 17 00:00:00 2001 From: Marcelo Salloum Date: Thu, 16 Apr 2026 11:51:19 -0700 Subject: [PATCH 3/4] cap XDR transaction fields at 8 KB in both schemas --- sdk/src/channel/Methods.test.ts | 21 +++++++++++++++++++++ sdk/src/channel/Methods.ts | 3 ++- sdk/src/charge/Methods.test.ts | 17 +++++++++++++++++ sdk/src/charge/Methods.ts | 6 +++++- sdk/src/shared/defaults.ts | 2 ++ 5 files changed, 47 insertions(+), 2 deletions(-) diff --git a/sdk/src/channel/Methods.test.ts b/sdk/src/channel/Methods.test.ts index d3b4392..9dfb9a7 100644 --- a/sdk/src/channel/Methods.test.ts +++ b/sdk/src/channel/Methods.test.ts @@ -141,4 +141,25 @@ describe('channel method schema', () => { }), ).toThrow() }) + + it('credential payload rejects oversized transaction XDR in open action', () => { + expect(() => + channel.schema.credential.payload.parse({ + action: 'open', + transaction: 'A'.repeat(8193), + amount: '1000000', + signature: 'a'.repeat(128), + }), + ).toThrow() + }) + + it('credential payload accepts transaction XDR at the size limit in open action', () => { + const result = channel.schema.credential.payload.parse({ + action: 'open', + transaction: 'A'.repeat(8192), + amount: '1000000', + signature: 'a'.repeat(128), + }) + expect(result.action).toBe('open') + }) }) diff --git a/sdk/src/channel/Methods.ts b/sdk/src/channel/Methods.ts index 9ac79b4..9df2832 100644 --- a/sdk/src/channel/Methods.ts +++ b/sdk/src/channel/Methods.ts @@ -1,5 +1,6 @@ import { Method } from 'mppx' import { z } from 'zod/mini' +import { DEFAULT_MAX_XDR_LENGTH } from '../shared/defaults.js' /** * Stellar one-way payment channel intent. @@ -20,7 +21,7 @@ export const channel = Method.from({ /** Action discriminator — open the channel on-chain. */ action: z.literal('open'), /** Signed channel-open transaction XDR (base64). */ - transaction: z.string(), + transaction: z.string().check(z.maxLength(DEFAULT_MAX_XDR_LENGTH)), /** Initial commitment amount in base units (stroops). */ amount: z.string().check(z.regex(/^\d+$/)), /** Ed25519 signature over the initial commitment bytes (128 hex chars). */ diff --git a/sdk/src/charge/Methods.test.ts b/sdk/src/charge/Methods.test.ts index 5846a3a..9afc0ec 100644 --- a/sdk/src/charge/Methods.test.ts +++ b/sdk/src/charge/Methods.test.ts @@ -184,4 +184,21 @@ describe('Methods.charge', () => { expect(result.transaction).toBe('AAAA...') } }) + + it('credential payload rejects oversized transaction XDR', () => { + expect(() => + Methods.charge.schema.credential.payload.parse({ + type: 'transaction', + transaction: 'A'.repeat(8193), + }), + ).toThrow() + }) + + it('credential payload accepts transaction XDR at the size limit', () => { + const result = Methods.charge.schema.credential.payload.parse({ + type: 'transaction', + transaction: 'A'.repeat(8192), + }) + expect(result.type).toBe('transaction') + }) }) diff --git a/sdk/src/charge/Methods.ts b/sdk/src/charge/Methods.ts index 6393375..bee624b 100644 --- a/sdk/src/charge/Methods.ts +++ b/sdk/src/charge/Methods.ts @@ -1,5 +1,6 @@ import { Method } from 'mppx' import { z } from 'zod/mini' +import { DEFAULT_MAX_XDR_LENGTH } from '../shared/defaults.js' /** * Stellar charge intent for one-time SEP-41 token transfers. @@ -23,7 +24,10 @@ export const charge = Method.from({ /** Push mode: client broadcasts and sends the tx hash. */ z.object({ hash: z.string().check(z.regex(/^[0-9a-f]{64}$/i)), type: z.literal('hash') }), /** Pull mode: client sends signed XDR as `payload.transaction`, server broadcasts. */ - z.object({ transaction: z.string(), type: z.literal('transaction') }), + z.object({ + transaction: z.string().check(z.maxLength(DEFAULT_MAX_XDR_LENGTH)), + type: z.literal('transaction'), + }), ]), }, request: z.object({ diff --git a/sdk/src/shared/defaults.ts b/sdk/src/shared/defaults.ts index 0d0d4a0..7ca47d6 100644 --- a/sdk/src/shared/defaults.ts +++ b/sdk/src/shared/defaults.ts @@ -1,4 +1,6 @@ export const DEFAULT_MAX_FEE_BUMP_STROOPS = 10_000_000 +/** Maximum base64-encoded XDR length accepted in credential payloads (~8 KB). */ +export const DEFAULT_MAX_XDR_LENGTH = 8_192 export const DEFAULT_POLL_MAX_ATTEMPTS = 20 export const DEFAULT_POLL_DELAY_MS = 1_000 export const DEFAULT_POLL_BACKOFF_MULTIPLIER = 1.2 From bbb7a1e04008ee061fa632729eac617b20fc3b01 Mon Sep 17 00:00:00 2001 From: Marcelo Salloum Date: Fri, 17 Apr 2026 12:06:57 -0700 Subject: [PATCH 4/4] update changelog for #44 --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 454a7a0..085b970 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +- Fix push-mode DoS via semaphore exhaustion: replace unbounded polling with single `getTransaction` lookup, add schema-level XDR size cap and hash format validation ([#44](https://github.com/stellar/stellar-mpp-sdk/pull/44)) + ## [0.5.0] - 2026-04-13 - Harden verification, replay protection, fix sponsored charge path, and replace SAC terminology with SEP-41 across docs, comments, and error messages ([#42](https://github.com/stellar/stellar-mpp-sdk/pull/42))