Address audit findings in export-and-sign/import by ethankonk · Pull Request #117 · tkhq/frames

ethankonk · 2026-03-04T21:41:24Z

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

andrewkmin

a couple gaps in tkintern's implementation 😛

main takeaways:

reduce the scope of this to exclude origin checks (perhaps in a separate PR)
on constant time helper function: let's DRY it up, fix/test the implementation, and move it to shared
we could explore creating a new class for the inMemoryKeys object and override the delete method (instead of calling zeroKeyEntry()). don't feel too strongly here

andrewkmin · 2026-03-09T16:21:25Z

+    let diff = 1;
+    const len = Math.min(aBuf.length, bBuf.length);
+    for (let i = 0; i < len; i++) {
+      diff |= aBuf[i] ^ bBuf[i];


This one is a valid finding -- we don't do anything with the diff that we mutate

andrewkmin · 2026-03-09T16:22:03Z

+    let diff = 1; // already know they differ
+    const len = Math.min(aBuf.length, bBuf.length);
+    for (let i = 0; i < len; i++) {
+      diff |= aBuf[i] ^ bBuf[i];


same here -- we should also DRY this up (write once, then import)

andrewkmin · 2026-03-09T16:29:41Z

 */
 function initMessageEventListener(HpkeDecrypt) {
  return async function messageEventListener(event) {
+    // SECURITY: Validate event.origin against the allowlist captured at init time.


let's remove this from the scope of this change for now

andrewkmin · 2026-03-09T16:29:54Z

        event.data["type"] == "TURNKEY_INIT_MESSAGE_CHANNEL" &&
        event.ports?.[0]
      ) {
+        // SECURITY: Capture the parent origin from the init handshake.


same with this. I think we can ticket it / consider tackling in a separate workstream

andrewkmin · 2026-03-09T16:30:24Z

-  if (enclaveQuorumPublic !== TURNKEY_SIGNER_ENCLAVE_QUORUM_PUBLIC_KEY) {
+  // SECURITY: Use constant-time comparison to prevent timing side-channel
+  // attacks that could leak the enclave quorum public key byte-by-byte.
+  if (


this is public information; we can remove the timingSafeEqual check

andrewkmin · 2026-03-09T16:33:27Z

+    let diff = 1;
+    const len = Math.min(aBuf.length, bBuf.length);
+    for (let i = 0; i < len; i++) {
+      diff |= aBuf[i] ^ bBuf[i];


same comment as in the other iframe: let's revisit this logic

andrewkmin · 2026-03-09T16:34:00Z

+ * @param {string} b
+ * @returns {boolean} true if strings are equal
+ */
+function timingSafeEqual(a, b) {


actually on second thought, we can fix this implementation and eventually throw it into https://github.com/tkhq/frames/tree/main/shared

andrewkmin · 2026-03-09T16:34:31Z

      // remove the message event listener that was added in the DOMContentLoaded event
      messageListenerController.abort();

+      // Capture the parent origin for use as targetOrigin in sendMessageUp,


want to roll this out separately ideally (just to reduce blast radius)

andrewkmin · 2026-03-09T16:35:55Z

+    // The decrypted payload is a raw 32-byte P-256 private key scalar.
+    const decrypted = await decryptBundle(bundle, organizationId, HpkeDecrypt);
+    keyBytes =
+      decrypted instanceof Uint8Array ? decrypted : new Uint8Array(decrypted);


ah, this accentuates the need for type safety :')

andrewkmin · 2026-03-09T16:37:16Z

+    }
+  } finally {
+    // SECURITY: Zero the raw private key bytes immediately after encoding.
+    // The encoded `key` is a JS string (hex or base58) which we cannot zero,


maybe we shouuuld rearchitect such that no private key material is ever a JS string? will have to think about this one; ultimately if it's displayed as a string in an iframe (which it sort of needs to be in order to be a functional export iframe), then we don't gain much from this

Copilot

Pull request overview

This PR aims to address multiple security audit findings in the export-and-sign and import flows by hardening comparisons, reducing key material lifetime/references in memory, and ensuring sensitive buffers are wiped, with corresponding rebuilds of dist artifacts and expanded tests.

Changes:

Added a shared timingSafeEqual helper and surfaced it through the export-and-sign TKHQ wrapper for timing-safe org ID checks.
Hardened export-and-sign key handling (in-place key map updates, buffer zeroing on success/error paths, and safer clearing semantics) and added tests covering these behaviors.
Updated built dist/ HTML and bundle outputs (new chunk hashes, license notices, updated sourcemaps).

Reviewed changes

Copilot reviewed 4 out of 16 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
shared/turnkey-core.js	Adds `timingSafeEqual`, updates embedded-key TTL comment block, tweaks `validateStyles` signature, exports new helper.
import/dist/standalone.html	Updates script chunk reference for rebuilt import standalone bundle.
import/dist/index.html	Updates script chunk reference for rebuilt import bundle.
import/dist/551.bundle.49402ee939a00be74d76.js.map	Removes old sourcemap for replaced import chunk.
import/dist/551.bundle.332145a70ecbd5b7542a.js.LICENSE.txt	Adds license notice for regenerator-runtime in rebuilt import chunk.
import/dist/551.bundle.332145a70ecbd5b7542a.js	Rebuilt import chunk output (new hash + sourcemap pointer).
export-and-sign/src/turnkey-core.js	Exposes `timingSafeEqual` via the export-and-sign TKHQ surface.
export-and-sign/src/event-handlers.js	Implements in-memory key store hardening and buffer-zeroing paths; uses timing-safe org ID comparison.
export-and-sign/index.test.js	Adds/updates tests for zeroing + key clearing behaviors and injected embedded key handling.
export-and-sign/dist/index.html	Updates script reference for rebuilt export-and-sign bundle.
export-and-sign/dist/bundle.ac8b5a20e77c0f386275.js.LICENSE.txt	Adds license notice for regenerator-runtime in rebuilt export-and-sign bundle.
export-and-sign/dist/bundle.ac8b5a20e77c0f386275.js	New rebuilt export-and-sign bundle output.
export-and-sign/dist/bundle.6f3ad536a859e78bdbd5.js	Removes old export-and-sign bundle output (replaced by new hash).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

ethankonk force-pushed the ethan/security-audit branch 2 times, most recently from d46f620 to 1a8dac4 Compare March 4, 2026 21:43

ethankonk changed the title ~~Address critical audit findings in export-and-sign/import~~ Address audit findings in export-and-sign/import Mar 5, 2026

andrewkmin requested review from andrewkmin and Copilot and removed request for Copilot March 9, 2026 16:15

andrewkmin reviewed Mar 9, 2026

View reviewed changes

andrewkmin requested changes Mar 9, 2026

View reviewed changes

ethankonk force-pushed the ethan/security-audit branch from e12539b to ab6d04c Compare April 29, 2026 19:34

ethankonk requested a review from andrewkmin April 29, 2026 20:23

ethankonk force-pushed the ethan/security-audit branch from 2a7d76b to 717551e Compare April 29, 2026 21:14

ethankonk added 3 commits July 16, 2026 15:24

address critical audit findings in export-and-sign/import

5a79862

ran through claude + codex for initial code reviewal

0025664

addressed comments

867eaa4

ethankonk force-pushed the ethan/security-audit branch from 717551e to 867eaa4 Compare July 16, 2026 19:26

andrewkmin requested a review from Copilot July 17, 2026 03:27

Copilot started reviewing on behalf of andrewkmin July 17, 2026 03:28 View session

Copilot AI reviewed Jul 17, 2026

View reviewed changes

Comment thread shared/turnkey-core.js Outdated

Comment thread shared/turnkey-core.js Outdated

Comment thread shared/turnkey-core.js

Comment thread export-and-sign/src/event-handlers.js Outdated

ethankonk force-pushed the ethan/security-audit branch from 5e3239f to 1313e9e Compare July 17, 2026 14:25

copilot comments

1ddff52

ethankonk force-pushed the ethan/security-audit branch from 1313e9e to 1ddff52 Compare July 17, 2026 14:55

Uh oh!

Conversation

ethankonk commented Mar 4, 2026

Uh oh!

andrewkmin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants