Skip to content

Address audit findings in export-and-sign/import#117

Open
ethankonk wants to merge 2 commits intomainfrom
ethan/security-audit
Open

Address audit findings in export-and-sign/import#117
ethankonk wants to merge 2 commits intomainfrom
ethan/security-audit

Conversation

@ethankonk
Copy link
Contributor

  • Constant-time string comparison (timingSafeEqual) for org ID and enclave quorum public key checks to prevent timing side-channel attacks
  • In-place key map mutation instead of spread-copy to avoid multiplying key material references on the V8 heap
  • Zero sensitive Uint8Array buffers (secretKey, privateKeyBytes, pkcs8) on all code paths including error paths via finally blocks
  • Origin validation on incoming postMessage events using allowedOrigin captured during the TURNKEY_INIT_MESSAGE_CHANNEL handshake
  • Use captured parent origin as targetOrigin in sendMessageUp instead of wildcard '*' to prevent message eavesdropping
  • Reduce embedded key localStorage TTL from 48h to 4h to limit exposure window if storage is compromised
  • Add tests for all new security behaviors: timing-safe comparison, parent origin capture/send, TTL enforcement, key clearing with buffer zeroing, and origin-scoped postMessage

@ethankonk ethankonk force-pushed the ethan/security-audit branch from a07ea64 to d46f620 Compare March 4, 2026 21:43
@ethankonk ethankonk force-pushed the ethan/security-audit branch from d46f620 to 1a8dac4 Compare March 4, 2026 21:43
@ethankonk
Copy link
Contributor Author

Code review

Found 1 issue:

  1. Origin validation bypass via empty-string event.origin: The origin check if (allowedOrigin && event.origin && event.origin !== allowedOrigin) requires event.origin to be truthy. In browsers, event.origin is "" (empty string, falsy) for messages from sandboxed iframes (<iframe sandbox>), file:// pages, data: URIs, and blob: URLs. This means messages from null-origin contexts bypass the validation entirely, even after allowedOrigin has been captured during the handshake. The fix is to check allowedOrigin !== null instead of relying on truthiness: if (allowedOrigin !== null && event.origin !== allowedOrigin).

// operations. We skip validation only if allowedOrigin hasn't been set yet
// (i.e. during the initial handshake itself).
if (allowedOrigin && event.origin && event.origin !== allowedOrigin) {
TKHQ.logMessage(
`⚠️ Rejected message from unexpected origin: ${event.origin} (expected: ${allowedOrigin})`
);
return;

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@ethankonk ethankonk changed the title Address critical audit findings in export-and-sign/import Address audit findings in export-and-sign/import Mar 5, 2026
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