fix: bind attribution memo nonce to challenge ID

[brendanjryan]

Summary

Fixes transaction hash stealing vulnerability in push-mode (hash credential) payments, with defense-in-depth for transaction credentials.

In a very rare occurrence attackers could steal legitimate on-chain transaction hashes and submit them against a different challenge, receiving services for free while locking out the legitimate payer.

Breaking changes

• Attribution.encode() now requires challengeId — old callers without it will get a type error
• Old clients that generate random attribution nonces or plain transfers will be rejected by new servers in push mode

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/mppx@291

commit: de0c99e

[tempoxyz-bot]

👁️ Cyclops Review

Note: Head drift detected — audited commit 1af4502 differs from current head 9a3238c. Findings have been re-checked against the current diff. Earlier findings about the missing transaction-path check, the custom memo regression, and the missing server fingerprint verification are resolved on the current head and omitted from inline comments.

Surviving Findings

⚠️ [ISSUE] Receipt-global memo scan allows unrelated TransferWithMemo log to satisfy challenge binding — assertChallengeBoundMemo scans all memo logs in the receipt rather than verifying the nonce on the specific log(s) that satisfied assertTransferLogs. A multi-call transaction with a same-currency dust transfer carrying a different challenge's nonce can satisfy both challenges independently if their hash stores aren't shared. See inline comment.

🛡️ [DEFENSE-IN-DEPTH] Deterministic challenge.id weakens memo-based replay protection (src/Challenge.ts:612-625, not in this diff) — challenge.id is HMAC(secret, realm|method|intent|request|expires|...) with no per-issuance randomness. If two 402 responses share the same expires (e.g., issued within the same millisecond), they produce identical IDs, and the memo nonce binding provides no replay resistance between them. Consider injecting a random value into opaque to ensure per-issuance uniqueness.

Reviewer Callouts

• ⚡ Receipt/log binding: The new defense proves some memo in the receipt is challenge-bound, but not that the matched payment log carries that memo. Evaluate whether the dust-transfer vector is practical in production.
• ⚡ Store topology: The exploit is most practical when hash stores are isolated (Store.memory() per handler). Deployment guidance should recommend shared stores for routes accepting the same payment parameters.
• ⚡ Challenge ID uniqueness: The deterministic challenge ID predates this PR. If challenge.id is now a security nonce, adding per-issuance randomness is worth a follow-up.

Stale Findings (fixed on current head)

• Transaction replay bypass via transaction payloads — assertChallengeBoundMemo now called in both paths.
• Custom memo regression — if (!memo) guard preserves custom memo support.
• Missing server fingerprint check — verifyServer() now called before accepting a memo log.

[tempoxyz-bot]

⚠️ [ISSUE] Receipt-global memo scan allows unrelated TransferWithMemo log to satisfy challenge binding

This memoLogs.some(...) iterates over ALL TransferWithMemo logs in the receipt, accepting any log whose token matches currency and whose nonce matches challengeId. It does not verify that the accepted memo log is the same log that satisfied assertTransferLogs.

A single multi-call transaction can contain: (1) the real merchant payment for challenge A with A's nonce, and (2) a same-currency dust transfer to an arbitrary address carrying challenge B's nonce. If the two challenges' hash stores are not shared (e.g., separate Store.memory() per handler), the same tx hash satisfies both challenges.

Recommended Fix: Return the matched log(s) from assertTransferLogs and require assertChallengeBoundMemo to verify the nonce on those exact logs rather than scanning the entire receipt.