Skip to content

Commit ac8262d

Browse files
authored
fix: defense in depth -- fail-closed expiry enforcement in credential verification (#262)
* fix: fail-closed expiry enforcement in credential verification Previously, credentials with no expires field were silently accepted because the check used `if (expires && ...)` which skips when expires is undefined. Now the handler rejects credentials missing expires with an InvalidChallengeError before checking the timestamp. Method-specific verify functions (Stripe, Tempo) also enforce fail-closed. Adds test for missing-expires rejection. MPP-F4 * test: add malformed expires guard and test - Reject credentials with unparseable expires (NaN date) as InvalidChallengeError - Guards added to core handler, Stripe verify, and Tempo verify - Test: 'returns 402 when credential challenge has malformed expires' * style: fix formatter nit in Stripe Charge.ts * refactor: unify expiry checks into Expires.assert helper * fix: align Expires.assert types with InvalidChallengeError.Options
1 parent bef374b commit ac8262d

File tree

5 files changed

+121
-14
lines changed

5 files changed

+121
-14
lines changed

src/Expires.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,28 @@
1+
import { InvalidChallengeError, PaymentExpiredError } from './Errors.js'
2+
3+
/**
4+
* Asserts that `expires` is present, well-formed, and not in the past.
5+
*
6+
* Throws `InvalidChallengeError` when missing or malformed,
7+
* and `PaymentExpiredError` when the timestamp is in the past.
8+
*/
9+
export function assert(
10+
expires: string | undefined,
11+
challengeId?: string,
12+
): asserts expires is string {
13+
if (!expires)
14+
throw new InvalidChallengeError({
15+
...(challengeId && { id: challengeId }),
16+
reason: 'missing required expires field',
17+
})
18+
if (Number.isNaN(new Date(expires).getTime()))
19+
throw new InvalidChallengeError({
20+
...(challengeId && { id: challengeId }),
21+
reason: 'malformed expires timestamp',
22+
})
23+
if (new Date(expires) < new Date()) throw new PaymentExpiredError({ expires })
24+
}
25+
126
/** Returns an ISO 8601 datetime string `n` days from now. */
227
export function days(n: number) {
328
return new Date(Date.now() + n * 24 * 60 * 60 * 1000).toISOString()

src/server/Mppx.test.ts

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,92 @@ describe('request handler', () => {
422422
`)
423423
expect((body as { detail: string }).detail).toContain('Payment expired at')
424424
})
425+
test('returns 402 when credential challenge has no expires (fail-closed)', async () => {
426+
const handle = Mppx.create({ methods: [method], realm, secretKey }).charge({
427+
amount: '1000',
428+
currency: asset,
429+
expires: new Date(Date.now() + 60_000).toISOString(),
430+
recipient: accounts[0].address,
431+
})
432+
433+
// Get a valid challenge from the server to capture the exact request shape
434+
const firstResult = await handle(new Request('https://example.com/resource'))
435+
expect(firstResult.status).toBe(402)
436+
if (firstResult.status !== 402) throw new Error()
437+
438+
const serverChallenge = Challenge.fromResponse(firstResult.challenge)
439+
440+
// Re-create the same challenge WITHOUT expires, with a valid HMAC
441+
const { expires: _, ...rest } = serverChallenge
442+
const challengeNoExpires = Challenge.from({
443+
secretKey,
444+
realm: rest.realm,
445+
method: rest.method,
446+
intent: rest.intent,
447+
request: rest.request,
448+
...(rest.opaque && { meta: rest.opaque }),
449+
})
450+
451+
const credential = Credential.from({
452+
challenge: challengeNoExpires,
453+
payload: { signature: '0x123', type: 'transaction' },
454+
})
455+
456+
const result = await handle(
457+
new Request('https://example.com/resource', {
458+
headers: { Authorization: Credential.serialize(credential) },
459+
}),
460+
)
461+
462+
expect(result.status).toBe(402)
463+
if (result.status !== 402) throw new Error()
464+
465+
const body = (await result.challenge.json()) as { title: string; detail: string }
466+
expect(body.title).toBe('Invalid Challenge')
467+
expect(body.detail).toContain('missing required expires')
468+
})
469+
test('returns 402 when credential challenge has malformed expires', async () => {
470+
const handle = Mppx.create({ methods: [method], realm, secretKey }).charge({
471+
amount: '1000',
472+
currency: asset,
473+
expires: new Date(Date.now() + 60_000).toISOString(),
474+
recipient: accounts[0].address,
475+
})
476+
477+
// Get a valid challenge from the server to capture the exact request shape
478+
const firstResult = await handle(new Request('https://example.com/resource'))
479+
expect(firstResult.status).toBe(402)
480+
if (firstResult.status !== 402) throw new Error()
481+
482+
const serverChallenge = Challenge.fromResponse(firstResult.challenge)
483+
484+
// Re-create the challenge with a valid HMAC but inject a malformed expires
485+
// by patching the challenge object after construction (bypasses zod at build time).
486+
const challengeMalformed = {
487+
...serverChallenge,
488+
expires: 'not-a-timestamp',
489+
}
490+
491+
const credential = Credential.from({
492+
challenge: challengeMalformed as any,
493+
payload: { signature: '0x123', type: 'transaction' },
494+
})
495+
496+
// Credential.serialize does not re-validate, so the malformed expires
497+
// reaches the server. Deserialization rejects it via zod schema.
498+
const result = await handle(
499+
new Request('https://example.com/resource', {
500+
headers: { Authorization: Credential.serialize(credential) },
501+
}),
502+
)
503+
504+
expect(result.status).toBe(402)
505+
if (result.status !== 402) throw new Error()
506+
507+
const body = (await result.challenge.json()) as { title: string; detail: string }
508+
expect(body.title).toBe('Malformed Credential')
509+
})
510+
425511
test('returns 402 when payload schema validation fails', async () => {
426512
const handle = Mppx.create({ methods: [method], realm, secretKey }).charge({
427513
amount: '1000',

src/server/Mppx.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -418,14 +418,14 @@ function createMethodFn(parameters: createMethodFn.Parameters): createMethodFn.R
418418
}
419419
}
420420

421-
// Reject expired credentials
422-
if (credential.challenge.expires && new Date(credential.challenge.expires) < new Date()) {
421+
// Reject credentials without expires (fail-closed) or with expired timestamp
422+
try {
423+
Expires.assert(credential.challenge.expires, credential.challenge.id)
424+
} catch (error) {
423425
const response = await transport.respondChallenge({
424426
challenge,
425427
input,
426-
error: new Errors.PaymentExpiredError({
427-
expires: credential.challenge.expires,
428-
}),
428+
error: error as Errors.PaymentError,
429429
})
430430
return { challenge: response, status: 402 }
431431
}

src/stripe/server/Charge.ts

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
import type * as Credential from '../../Credential.js'
2-
import {
3-
PaymentActionRequiredError,
4-
PaymentExpiredError,
5-
VerificationFailedError,
6-
} from '../../Errors.js'
2+
import { PaymentActionRequiredError, VerificationFailedError } from '../../Errors.js'
3+
import * as Expires from '../../Expires.js'
74
import type { LooseOmit, OneOf } from '../../internal/types.js'
85
import * as Method from '../../Method.js'
96
import type { StripeClient } from '../internal/types.js'
@@ -66,8 +63,7 @@ export function charge<const parameters extends charge.Parameters>(parameters: p
6663
const { challenge } = credential
6764
const { request } = challenge
6865

69-
if (challenge.expires && new Date(challenge.expires) < new Date())
70-
throw new PaymentExpiredError({ expires: challenge.expires })
66+
Expires.assert(challenge.expires, challenge.id)
7167

7268
const parsed = Methods.charge.schema.credential.payload.safeParse(credential.payload)
7369
if (!parsed.success) throw new Error('Invalid credential payload: missing or malformed spt')

src/tempo/server/Charge.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import {
99
import { tempo as tempo_chain } from 'viem/chains'
1010
import { Abis, Transaction } from 'viem/tempo'
1111

12-
import { PaymentExpiredError } from '../../Errors.js'
12+
import * as Expires from '../../Expires.js'
1313
import type { LooseOmit, NoExtraKeys } from '../../internal/types.js'
1414
import * as Method from '../../Method.js'
1515
import * as Store from '../../Store.js'
@@ -118,7 +118,7 @@ export function charge<const parameters extends charge.Parameters>(
118118
const currency = challengeRequest.currency as `0x${string}`
119119
const recipient = challengeRequest.recipient as `0x${string}`
120120

121-
if (expires && new Date(expires) < new Date()) throw new PaymentExpiredError({ expires })
121+
Expires.assert(expires, challenge.id)
122122

123123
const memo = methodDetails?.memo as `0x${string}` | undefined
124124

0 commit comments

Comments
 (0)