Skip to content

Fix: webhook timing-safety + tokenization double-submit#2

Merged
BenKalsky merged 2 commits into
mainfrom
fix/webhook-timing-safety-and-double-submit
May 1, 2026
Merged

Fix: webhook timing-safety + tokenization double-submit#2
BenKalsky merged 2 commits into
mainfrom
fix/webhook-timing-safety-and-double-submit

Conversation

@BenKalsky
Copy link
Copy Markdown
Member

Summary

Addresses the two High-severity findings from a recent code review.

1. Webhook secret length leak via timing oracle

`verifySumitSharedSecret` previously called a `timingSafeEqual` that returned early on length mismatch — a classic timing oracle that lets an attacker measure response latency to learn the byte-length of the configured webhook secret. Replaced with a SHA-256 comparison: both inputs are hashed to a fixed-length 32-byte digest, then compared in a constant-time loop. The comparison now leaks neither content nor length.

Implementation uses Web Crypto (`crypto.subtle.digest`) so the route helper continues to work in both Node and Edge runtimes.

2. Tokenization double-submit race

`` guarded against double submission with `if (submitting) return`, where `submitting` is React state. Two rapid clicks (or `submit()` calls) both pass the guard before the first `setSubmitting(true)` re-renders, so both fire `sdk.CreateToken`. Single-use tokens are exactly that — single-use — so this race could waste tokens.

Replaced with a synchronous `useRef` flag (`submittingRef.current`) that is updated immediately. The visible `submitting` state remains for the disabled-input UI.

Bonus polish

  • New `CLAUDE.md` covering architecture, security model, and conventions.
  • README badges and a new Security table.
  • New `SumitCheckout.test.tsx` regression test for the double-submit guard.
  • Webhook test for length-independent rejection of wrong secrets.

Test plan

  • pnpm test — 21/21 passing (3 new tests)
  • pnpm typecheck clean
  • pnpm build clean

🤖 Generated with Claude Code

BenKalsky and others added 2 commits May 1, 2026 15:35
- `verifySumitSharedSecret`: hash both inputs with SHA-256 (Web Crypto)
  before constant-time compare. The previous early-return on length
  mismatch leaked the secret's byte-length via response timing.
- `<SumitCheckout />`: replace the `submitting` state-based guard with a
  synchronous `useRef` flag so two rapid submits can't both fire
  `CreateToken` while React batches the state update (single-use tokens
  must stay single-use).
- Add tests covering both fixes.
- Add CLAUDE.md (architecture, security model, conventions).
- README: badges and a Security table summarising threat handling.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI failed at the setup-node step: `cache: pnpm` requires `pnpm-lock.yaml`,
but the repo gitignored it. Two changes here:

1. Stop ignoring `pnpm-lock.yaml` and commit it for reproducible installs.
2. Update the workflow to check out `Digitizers/sumit-api` as a sibling
   directory (sumit-react depends on `file:../sumit-api`) and build its
   dist/ before running sumit-react's typecheck and tests. Once sumit-api
   ships to npm, the second checkout step can be removed and the dev
   dependency switched to a regular semver range.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@BenKalsky BenKalsky merged commit b416eec into main May 1, 2026
1 check passed
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.

1 participant