Skip to content

fix(x402-core): bind signature to prepared transaction - #56

Open
nikhilkumar1612 wants to merge 3 commits into
ChainSafe:mainfrom
nikhilkumar1612:fix/tx-hash
Open

nikhilkumar1612 wants to merge 3 commits into
ChainSafe:mainfrom
nikhilkumar1612:fix/tx-hash

Conversation

@nikhilkumar1612

Copy link
Copy Markdown

Summary

Recompute the canonical Canton hash from the submitted prepared transaction during verification and reject payloads whose client-supplied hash does not match.

This binds the payer’s signature to the exact transaction that the facilitator will submit, preventing attackers from selecting arbitrary hashes or bypassing
hash-derived replay identities.

Closes #55.

Changes

  • Compute the canonical prepared-transaction hash using @canton-network/core-tx-visualizer.
  • Compare the computed hash with preparedTransactionHash.
  • Verify the payer signature against the trusted, recomputed hash.
  • Add prepared_transaction_hash_mismatch as a verification failure reason.
  • Fail closed for unsupported hashing schemes; canonical hashing currently supports V2.
  • Make the verifier interface asynchronous to support canonical hashing.
  • Add a parity test using a prepared transaction and hash produced by a real Canton prepare call.
  • Add a regression test for a correctly signed but unrelated hash.
  • Expand the synthetic prepared-transaction fixture with the transaction structure and metadata required by the canonical hasher.
  • Add a changeset for @chainsafe/x402-core.

Verification

  • pnpm --filter @chainsafe/x402-core test
  • Canonical V2 hash matches the hash returned by a real Canton prepare call
  • Mismatched transaction/hash payload is rejected
  • Verified against Splice localnet

@nikhilkumar1612

Copy link
Copy Markdown
Author

@dhyaniarun1993 @sqhell @dimartiro can anyone help me with a review please ? 🙏

// 9. Derive the trusted hash from the submitted prepared transaction.
let computedHash: string;
try {
computedHash = await computePreparedTransactionHash(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocker (critical): pre-auth DoS. This PR moves the hash recomputation (step 9) ahead of signature verification (step 11, line 195). The core-tx-visualizer hasher (encodeNodeId → encodeNode → encodeRepeated(exercise.children, …)) walks the node graph recursively with no visited set and no memoization, so a preparedTransaction whose nodes reference one another produces work that is super-linear in the body size and can exhaust the process heap (fatal OOM, not caught by the try/catch here since there is no thrown exception).

And it is reachable without a valid signature: steps 1–8 are all satisfiable offline (requirementsHash is a pure function of requirements that travel in the same body; the fingerprint check compares against a key the client itself supplies). In the facilitator the auth middleware on POST /v2/verify is optional and the 1 MB body limit leaves ample room below the dangerous threshold, so the rate limit does not protect against it.

On main this work did not exist pre-auth (the in-house decoder, prepared-tx.ts, is single-pass and ran after the signature). Bumping to 1.11.1 does not fix it: same recursion upstream.

Fix (in this PR): before calling computePreparedTransactionHash, add a cheap structural guard on inner.preparedTransaction: a size cap on the blob, an iterative walk of the node graph with a Set of visited ids requiring a tree/acyclic shape (each node referenced as a child at most once), and a node-count cap. Fail closed with a specific reason if it does not pass. Worth reporting upstream to @canton-network/core-tx-visualizer as well.

@nikhilkumar1612

Copy link
Copy Markdown
Author

apologies for the delayed response @dimartiro
it should have the necessary fix now 🙏

Comment on lines +69 to +71
if (transaction.nodes.length > MAX_PREPARED_TRANSACTION_NODES) {
return { ok: false, reason: "node_limit" };
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, the graph check closes the recursion issue. One gap left: the guard bounds nodes but not transaction.nodeSeeds or metadata.inputContracts. The hasher's findSeed scans all seeds for every create/exercise node, so 1024 nodes plus ~40k dummy seeds (still under 256 KiB) take ~400 ms of hashing per request, before the signature check. ~15k empty input contracts take ~290 ms (one sha256 each). With these caps the worst case is ~37 ms. Canton emits at most one seed per node, so the first can't reject a real transaction, and 1024 input contracts leaves a lot of room over a real transfer.

Suggested change
if (transaction.nodes.length > MAX_PREPARED_TRANSACTION_NODES) {
return { ok: false, reason: "node_limit" };
}
if (transaction.nodes.length > MAX_PREPARED_TRANSACTION_NODES) {
return { ok: false, reason: "node_limit" };
}
// Canton emits at most one seed per node; the hasher scans all of them for each node.
if (transaction.nodeSeeds.length > transaction.nodes.length) {
return { ok: false, reason: "node_seed_limit" };
}
// Each input contract costs its own sha256 in the hasher.
if ((decoded.metadata?.inputContracts.length ?? 0) > MAX_PREPARED_TRANSACTION_INPUT_CONTRACTS) {
return { ok: false, reason: "input_contract_limit" };
}

Comment on lines +9 to +14
export const MAX_PREPARED_TRANSACTION_NODES = 1_024;

export type PreparedTransactionGuardFailure =
| "size_limit"
| "undecodable"
| "node_limit"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Goes with the suggestion on the node limit check further down.

Suggested change
export const MAX_PREPARED_TRANSACTION_NODES = 1_024;
export type PreparedTransactionGuardFailure =
| "size_limit"
| "undecodable"
| "node_limit"
export const MAX_PREPARED_TRANSACTION_NODES = 1_024;
export const MAX_PREPARED_TRANSACTION_INPUT_CONTRACTS = 1_024;
export type PreparedTransactionGuardFailure =
| "size_limit"
| "undecodable"
| "node_limit"
| "node_seed_limit"
| "input_contract_limit"

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(x402-core): the signature isn't bound to the prepared transaction — preparedTransactionHash is never verified

2 participants