[zahin-mohammad]

Problem and fix

Signing and distributed key generation left native WebAssembly sessions allocated after saving their state as bytes. The JavaScript reference was discarded without calling free(). In the installed library, newly constructed sessions have no automatic cleanup callback, so those allocations leak.

This PR frees sessions and first-round messages after copying their data. It also clears session references before cleanup and preserves the original signing error if cleanup fails. Final signature combination and key-share extraction already consume their sessions; those paths are protected from double-freeing.

Ticket: WCN-4341 · Customer report

Validation

• 6,000 complete signing cycles, split across original/fixed code with and without periodic forced garbage collection. Every signature matched between parties and passed cryptographic verification.
• 35 focused tests pass, covering signing, key generation, restored sessions, and cleanup failures, including simultaneous payload-read and message-disposal failures. Build, lint, formatting, and explicit type checking of both focused test files pass.
• Independent re-review found no further changes needed. GitHub checks and reviews must still be assessed on the latest commit.

The table shows memory capacity after 1,500 signing cycles per configuration. MiB means 1,048,576 bytes. WebAssembly capacity is a high-water mark, not live allocated bytes; the ownership probes below separately confirm the leak.

Code	Periodic forced garbage collection	Final WebAssembly capacity
Original	Every 100 cycles	484.12 MiB
Original	None during workload	1,621.44 MiB
Fixed	Every 100 cycles	2.31 MiB
Fixed	None during workload	2.31 MiB

The chart shows the configuration without periodic forced collection. Red: original. Blue: fixed.

%%{init: {"themeVariables": {"xyChart": {"plotColorPalette": "#c62828,#1565c0"}}}}%%
xychart-beta
    title "Signing: WebAssembly capacity without forced collection"
    x-axis "Verified signing cycles" [0,100,200,300,400,500,600,700,800,900,1000,1100,1200,1300,1400,1500]
    y-axis "Capacity (MiB)" 0 --> 1800
    line [1.75,109.56,217.50,325.44,433.81,541.25,649.31,757.19,865.25,973.06,1081.06,1189.50,1297.06,1405.00,1512.88,1621.44]
    line [1.75,2.31,2.31,2.31,2.31,2.31,2.31,2.31,2.31,2.31,2.31,2.31,2.31,2.31,2.31,2.31]

Loading

Limits: This reproduces the allocation leak, not the customer's exact 11-day crash. Tests used Node 22.23.4 and binding version 1.2.0-pre.4; the customer reported Node 22.22.0. Production concurrency, browser execution, and recovery after a native trap remain unverified. The separate decryption warning is not fixed here.

Additional finding

The audit found the same permanent leak in key generation, which this PR also fixes. Ten complete three-party workflows produced all 30 key shares. After the fix, the probe recorded 120 explicit session frees and 30 consuming calls, with no session cleanup deferred to automatic finalizers. Raw output and the probe are below.

Other audited algorithm bindings register automatic cleanup callbacks. Their probes showed delayed reclamation, not this missing-callback defect; they are outside this change.

Root cause and corrected ownership flow

A JavaScript wrapper holds a pointer to native protocol state. Serializing the state copies it into bytes but leaves the original allocation owned by the wrapper. Dropping that wrapper is safe only if its native allocation is explicitly released or a registered finalizer eventually releases it.

The installed binding registers restored wrappers in __wrap() but not directly constructed sessions (node_modules/@silencelaboratories/dkls-wasm-ll-node/dkls-wasm-ll-node.js:802, :827). That creates two distinct problems: constructor allocations are leaked, while restored-session allocations wait for garbage collection.

flowchart TD
    A[Create or restore native session] --> B[Process signing round]
    B --> C[Copy session state into bytes]
    C --> D[Free native session explicitly]
    D --> E[Next request restores from saved bytes]
    E --> B
    B --> F[Final signature combination]
    F --> G[Native combination consumes session]

Loading

The binding documents final combination as: “This method consumes the session and deallocates all internal data.” (dkls-wasm-ll-node.js:945). Key-share extraction also consumes its session. Calling free() again after either operation would be incorrect.

Implementation: modules/sdk-lib-mpc/src/tss/ecdsa-dkls/dsg.ts and modules/sdk-lib-mpc/src/tss/ecdsa-dkls/dkg.ts. Regression coverage: modules/sdk-lib-mpc/test/unit/tss/ecdsa/dklsDsg.ts and modules/sdk-lib-mpc/test/unit/tss/ecdsa/dklsDkg.ts.

Signing proof: raw output and reproducible harness

The original source is 368a7c1^. The tested fixed signing source has SHA-256 dcf003c0f70adab9762f230118c1e17310377590052dea9ceafefa1392d58ed5 and is included in follow-up commit 7cd4130.

Each configuration ran in a fresh process. One party was restored from serialized bytes before each subsequent round. The workload excludes HTTP transport and encryption. Final memory records follow forced garbage collection; already-grown WebAssembly capacity does not shrink afterward.

Raw proof

Actual final workload records (verbatim)

{"variant":"baseline","gcMode":"gc","cycle":1500,"phase":"after-final-gc","created":3000,"restored":12000,"freed":0,"consumed":3000,"wasmBytes":507641856,"rss":641056768,"heapTotal":62504960,"heapUsed":25697584,"external":514933899,"arrayBuffers":1017553}
{"variant":"baseline","gcMode":"nogc","cycle":1500,"phase":"after-final-gc","created":3000,"restored":12000,"freed":0,"consumed":3000,"wasmBytes":1700200448,"rss":1834844160,"heapTotal":63291392,"heapUsed":25701856,"external":1707492507,"arrayBuffers":1017569}
{"variant":"fixed","gcMode":"gc","cycle":1500,"phase":"after-final-gc","created":3000,"restored":12000,"freed":12000,"consumed":3000,"wasmBytes":2424832,"rss":138215424,"heapTotal":64077824,"heapUsed":25833952,"external":9716857,"arrayBuffers":1017535}
{"variant":"fixed","gcMode":"nogc","cycle":1500,"phase":"after-final-gc","created":3000,"restored":12000,"freed":12000,"consumed":3000,"wasmBytes":2424832,"rss":136200192,"heapTotal":63553536,"heapUsed":25809472,"external":9716870,"arrayBuffers":1017548}

Finalizer ownership probes (actual output)

A finalizer is an automatic cleanup callback registered for a JavaScript wrapper. These probes instrument registration, explicit disposal, consuming operations, and callback execution. Directly constructed sessions have no registration in this binding; restored sessions do.

finalizers-baseline.jsonl
{"runtime":"v22.23.4","dependency":"1.2.0-pre.4","variant":"baseline","gcMode":"gc","count":"10","mode":"load","sourceSha":"58da2d5bf5408ba380ce563067d6734bb72ebb28cc3323e28f700c51d97390c2","exports":["memory","__wbg_signsessionotvariant_free","__wbg_keygensession_free","__wbg_keyshare_free","__wbg_message_free","__wbg_signsession_free","__wbindgen_malloc","__wbindgen_realloc","__wbindgen_free"]}
{"variant":"baseline","gcMode":"gc","cycle":0,"phase":"start","created":0,"restored":0,"freed":0,"consumed":0,"wasmBytes":1835008,"rss":137003008,"heapTotal":38846464,"heapUsed":32300936,"external":5273323,"arrayBuffers":1401102}
{"variant":"baseline","gcMode":"gc","cycle":10,"phase":"before-final-gc","created":20,"restored":80,"freed":0,"consumed":20,"wasmBytes":12910592,"rss":177160192,"heapTotal":73744384,"heapUsed":37560464,"external":21377880,"arrayBuffers":6382599}
{"variant":"baseline","gcMode":"gc","cycle":10,"phase":"after-final-gc","created":20,"restored":80,"freed":0,"consumed":20,"wasmBytes":12910592,"rss":168656896,"heapTotal":65077248,"heapUsed":28619424,"external":20198948,"arrayBuffers":1009363}
{"finalizers":{"keygensession":{"registered":0,"finalized":0},"keyshare":{"registered":21,"finalized":0},"message":{"registered":80,"finalized":20},"signsession":{"registered":0,"finalized":0},"signsessionotvariant":{"registered":80,"finalized":60}}}

finalizers-fixed.jsonl
{"runtime":"v22.23.4","dependency":"1.2.0-pre.4","variant":"fixed","gcMode":"gc","count":"10","mode":"load","sourceSha":"dcf003c0f70adab9762f230118c1e17310377590052dea9ceafefa1392d58ed5","exports":["memory","__wbg_signsessionotvariant_free","__wbg_keygensession_free","__wbg_keyshare_free","__wbg_message_free","__wbg_signsession_free","__wbindgen_malloc","__wbindgen_realloc","__wbindgen_free"]}
{"variant":"fixed","gcMode":"gc","cycle":0,"phase":"start","created":0,"restored":0,"freed":0,"consumed":0,"wasmBytes":1835008,"rss":136495104,"heapTotal":38846464,"heapUsed":32567672,"external":5273323,"arrayBuffers":1401102}
{"variant":"fixed","gcMode":"gc","cycle":10,"phase":"before-final-gc","created":20,"restored":80,"freed":80,"consumed":20,"wasmBytes":2424832,"rss":170459136,"heapTotal":79134720,"heapUsed":41444632,"external":10261008,"arrayBuffers":5751487}
{"variant":"fixed","gcMode":"gc","cycle":10,"phase":"after-final-gc","created":20,"restored":80,"freed":80,"consumed":20,"wasmBytes":2424832,"rss":157450240,"heapTotal":64815104,"heapUsed":28622104,"external":9713195,"arrayBuffers":1009370}
{"finalizers":{"keygensession":{"registered":0,"finalized":0},"keyshare":{"registered":21,"finalized":0},"message":{"registered":80,"finalized":0},"signsession":{"registered":0,"finalized":0},"signsessionotvariant":{"registered":80,"finalized":0}}}

full-keygen.jsonl
{"cycles":10,"partiesPerCycle":3,"explicitSessionFrees":0,"consumingKeyshareCalls":30,"counts":{"keygensession":{"registered":90,"finalized":60},"keyshare":{"registered":30,"finalized":0},"message":{"registered":180,"finalized":30},"signsession":{"registered":0,"finalized":0},"signsessionotvariant":{"registered":0,"finalized":0}},"wasmBytes":9633792}

Reproducible workload

Run from the repository root after building modules/sdk-lib-mpc. Save the following harness as /tmp/wcn4341-run.cjs. The harness reads baseline source through Git and transpiles each variant in memory; it does not replace working source. It records source hashes, runtime, dependency version, memory counters and session-ownership counts. The fixed mode always reads the current working signing source, so compare its logged hash to the tested hash above.

node_modules/.bin/tsc --build modules/sdk-lib-mpc
# Each command runs in a separate Node 22.23.4 process.
node --expose-gc /tmp/wcn4341-run.cjs baseline gc 1500
node --expose-gc /tmp/wcn4341-run.cjs baseline nogc 1500
node --expose-gc /tmp/wcn4341-run.cjs fixed gc 1500
node --expose-gc /tmp/wcn4341-run.cjs fixed nogc 1500

Complete workload harness

const fs=require('fs'),cp=require('child_process'),Module=require('module'),path=require('path'),assert=require('assert'),crypto=require('crypto');
const root=process.cwd(), req=Module.createRequire(root+'/package.json'),ts=req('typescript');
const [variant='fixed',gcMode='gc',count='1500',mode='load']=process.argv.slice(2);
const sourcePath=root+'/modules/sdk-lib-mpc/src/tss/ecdsa-dkls/dsg.ts';
const source=variant==='baseline'?cp.execFileSync('git',['show','368a7c1^:modules/sdk-lib-mpc/src/tss/ecdsa-dkls/dsg.ts'],{encoding:'utf8'}):fs.readFileSync(sourcePath,'utf8');
const compile=(src,name)=>{const m=new Module(name,module);m.filename=name;m.paths=Module._nodeModulePaths(path.dirname(name));m._compile(ts.transpileModule(src,{compilerOptions:{module:ts.ModuleKind.CommonJS,target:ts.ScriptTarget.ES2020,esModuleInterop:true}}).outputText,name);return m.exports;};
// The synthetic module resolves sibling types from the existing built tree; no source file is overwritten.
const {Dsg}=compile(source,root+'/modules/sdk-lib-mpc/dist/src/tss/ecdsa-dkls/load-isolated.cjs');
const wasm=req('@silencelaboratories/dkls-wasm-ll-node'),{decode}=req('cbor-x');
let created=0,restored=0,freed=0,consumed=0;
const C=wasm.SignSessionOTVariant,oldFrom=C.fromBytes,oldFree=C.prototype.free,oldCombine=C.prototype.combine;
C.fromBytes=function(...a){restored++;return oldFrom.apply(this,a)};
C.prototype.free=function(...a){freed++;return oldFree.apply(this,a)};
C.prototype.combine=function(...a){consumed++;return oldCombine.apply(this,a)};
const instrumented={...wasm,SignSessionOTVariant:class extends C{constructor(...a){super(...a);created++}}};
const fixture=root+'/modules/sdk-lib-mpc/test/unit/tss/ecdsa/fixtures/';
const shares=['userShare','backupShare'].map(n=>fs.readFileSync(fixture+n));
const key=wasm.Keyshare.fromBytes(shares[0]);const pub=Buffer.from(key.publicKey).toString('hex')+Buffer.from(decode(key.toBytes()).root_chain_code).toString('hex');key.free();
const {verifyAndConvertDklsSignature}=req(root+'/modules/sdk-lib-mpc/dist/src/tss/ecdsa-dkls/util.js');
const make=(i,hash)=>new Dsg(shares[i],i,'m',hash,instrumented);
const sample=(cycle,phase)=>console.log(JSON.stringify({variant,gcMode,cycle,phase,created,restored,freed,consumed,wasmBytes:wasm.__wasm.memory.buffer.byteLength,...process.memoryUsage()}));
(async()=>{console.log(JSON.stringify({runtime:process.version,dependency:req('@silencelaboratories/dkls-wasm-ll-node/package.json').version,variant,gcMode,count,mode,sourceSha:crypto.createHash('sha256').update(source).digest('hex'),exports:Object.keys(wasm.__wasm).filter(k=>/alloc|free|memory/.test(k))}));
if(mode==='malformed'||mode==='empty-payload'||mode.startsWith('cbor:')){const p=make(0,Buffer.alloc(32));await p.init();try{p.handleIncomingMessages({broadcastMessages:(mode==='empty-payload'||mode.startsWith('cbor:'))?[{payload:Buffer.from(mode.split(':')[1]||'', 'hex'),from:1}]:[],p2pMessages:[]});console.log('UNEXPECTED_SUCCESS')}catch(e){console.log(JSON.stringify({error:String(e),sessionCleared:p.dsgSession===undefined}));}return;}
sample(0,'start');
if(mode==='trap'){const hash=Buffer.alloc(32);const a=make(0,hash),b=make(1,hash);let ms=await Promise.all([a.init(),b.init()]);ms=ms.map(m=>({broadcastMessages:[m],p2pMessages:[]}));for(let r=2;r<=3;r++)ms=[a,b].map((p,i)=>p.handleIncomingMessages(ms[1-i]));a.messageHash=Buffer.alloc(0);try{a.handleIncomingMessages(ms[1]);console.log('UNEXPECTED_SUCCESS')}catch(e){console.log(JSON.stringify({error:String(e),sessionCleared:a.dsgSession===undefined}));}return;}
for(let cycle=1;cycle<=+count;cycle++){
 const msg=Buffer.from('public-load-test-'+cycle),hash=crypto.createHash('sha256').update(msg).digest();let parties=[make(0,hash),make(1,hash)];
 let messages=(await Promise.all(parties.map(p=>p.init()))).map(m=>({broadcastMessages:[m],p2pMessages:[]}));
 for(let round=2;round<=5;round++){
  // Party zero models an external signer: reconstruct its JS wrapper from serialized state every round.
  const state=parties[0].getSession();parties[0]=make(0,hash);await parties[0].setSession(state);
  messages=parties.map((p,i)=>p.handleIncomingMessages(messages[1-i]));
 }
 assert.deepStrictEqual(parties[0].signature,parties[1].signature);assert(verifyAndConvertDklsSignature(msg,parties[0].signature,pub,'m'));
 if(cycle%100===0){if(gcMode==='gc'){global.gc();await new Promise(r=>setImmediate(r));global.gc();await new Promise(r=>setImmediate(r));}sample(cycle,'running');}
}
sample(+count,'before-final-gc');for(let i=0;i<8;i++){global.gc();await new Promise(r=>setTimeout(r,10));}sample(+count,'after-final-gc');
})().catch(e=>{console.error(e);process.exitCode=1});

Key-generation proof: raw output and probe

Follow-up key-generation probe, actual output

{"cycles":10,"partiesPerCycle":3,"explicitSessionFrees":120,"consumingKeyshareCalls":30,"counts":{"keygensession":{"registered":120,"finalized":0},"keyshare":{"registered":30,"finalized":0},"message":{"registered":180,"finalized":0},"signsession":{"registered":0,"finalized":0},"signsessionotvariant":{"registered":0,"finalized":0}},"wasmBytes":1900544}

Invocation from repository root, Node 22.23.4: node --expose-gc full-keygen.cjs. This probe and the original ownership probes are investigation artifacts, not checked-in regression tests. The PR's accompanying tests cover the corresponding cleanup assertions.

Key-generation probe source

Save as full-keygen.cjs and run from the repository root after building the tested source.

const {createRequire}=require('module'),req=createRequire(process.cwd()+'/package.json');
const counts={};const FR=global.FinalizationRegistry;global.FinalizationRegistry=class extends FR{constructor(fn){const label=fn.toString().match(/__wbg_([a-z]+)_free/)?.[1]||'other';const c=counts[label]??={registered:0,finalized:0};super(p=>{c.finalized++;fn(p)});this.c=c}register(...a){this.c.registered++;return super.register(...a)}};
const w=req('@silencelaboratories/dkls-wasm-ll-node');let explicit=0,consumed=0;const oldFree=w.KeygenSession.prototype.free,oldKey=w.KeygenSession.prototype.keyshare;w.KeygenSession.prototype.free=function(){explicit++;return oldFree.call(this)};w.KeygenSession.prototype.keyshare=function(){consumed++;return oldKey.call(this)};
(async()=>{const {generateDKGKeyShares}=req(process.cwd()+'/modules/sdk-lib-mpc/dist/src/tss/ecdsa-dkls/util.js');for(let i=0;i<10;i++){const parties=await generateDKGKeyShares();if(parties.some(p=>!p.getKeyShare().length))throw Error('missing completed key share')}for(let j=0;j<8;j++){global.gc();await new Promise(r=>setTimeout(r,10))}console.log(JSON.stringify({cycles:10,partiesPerCycle:3,explicitSessionFrees:explicit,consumingKeyshareCalls:consumed,counts,wasmBytes:w.__wasm.memory.buffer.byteLength}))})().catch(e=>{console.error(e);process.exitCode=1});

Remaining questions, alternatives, and glossary

• Release owners must select the SDK and Express release versions. No release date is promised here.
• The separate native-decryption warning follows successful legacy fallback; its cause needs the complete error and sanitized ciphertext metadata. Version-two ciphertext routes directly to its own decryptor (modules/sdk-api/src/encrypt.ts:136, :167).
• Malformed-input probes preserved the original error and cleared wrappers, but did not reproduce the production native trap. Successful cleanup does not establish that a trapped native module can safely be reused.

Appendix: alternatives considered

Option	Ownership guarantee	Compatibility	Decision
Explicit cleanup after serialization	Deterministic disposal at round boundary	Preserves serialized-session interface	Selected
Wait for finalizers	Constructor-created sessions lack registration in this binding	No interface change	Reject
Keep live sessions across requests	Requires ownership across request lifetimes	Changes state-management design	Out of scope
Restart processes periodically	Limits process lifetime, not allocation defect	Requires validated draining and scheduling	Temporary mitigation only

Appendix: domain primer

Native allocation versus wrapper

The JavaScript wrapper holds a pointer to native protocol state. Dropping the reference does not call its destructor. The installed binding registers restored wrappers in __wrap() but not its direct constructor (node_modules/@silencelaboratories/dkls-wasm-ll-node/dkls-wasm-ll-node.js:802, :827). References: wasm-bindgen exported types, MDN finalization.

Serialized protocol state

Serialization copies enough state into bytes for another request to continue signing. It leaves ownership of the original allocation with the caller. Final signature combination and key-share extraction instead consume their sessions, so they must not be followed by another session destructor. References: modules/sdk-lib-mpc/src/tss/ecdsa-dkls/dsg.ts, modules/sdk-lib-mpc/src/tss/ecdsa-dkls/dkg.ts, WebAssembly memory.

Appendix: glossary

• RCA: Root-cause analysis, distinguishing verified defects from suspected incident consequences.
• Wasm: WebAssembly, the native cryptographic module's execution format.
• DKLS: Signing protocol family implemented by the dependency used here.
• MPCv2: BitGo's second-generation multiparty signing path.
• Dsg/Dkg: Distributed signing/key-generation wrapper classes.
• GC: Garbage collection, reclaiming unreachable JavaScript objects.
• RSS: Resident set size, the process's physical-memory footprint.
• MiB: Mebibyte, 1,048,576 bytes.
• Finalizer: Automatic cleanup callback associated with an unreachable wrapper.
• Consuming method: Method that takes ownership and destroys or transfers its native input.
• Envelope: Encoded ciphertext plus its version and cryptographic parameters.
• SHA-256: Cryptographic hash used here to identify the exact tested source.
• CI: Continuous integration, automated checks run for the pull request.

[linear-code]

WCN-4341

[github-actions]

⚠️ Compatibility checks are failing on Node.js 27-nightly (experimental, non-blocking). This does not block merge, but it signals an incompatibility to fix before this release becomes supported.

View run

[zahin-mohammad]

@claude review always

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Besides the inline finding, I checked two other things and neither is a problem. In init(), _deserializeState() only reads this.dsgSession and never reassigns it, so the finally frees the session created at dsg.ts:140. Reading firstMessage.payload and then freeing the message follows the same pattern the existing round handler already uses (nextRoundMessages.forEach((m) => m.free()) after m.payload). The binding is not installed in this checkout, so I did not confirm from its source that the payload getter returns a copy.

Extended reasoning...

The change adds explicit free() calls for DKLS WASM signing sessions and first-round messages in modules/sdk-lib-mpc/src/tss/ecdsa-dkls/dsg.ts, plus a unit test that counts the frees. It touches MPC/ECDSA signing code, which is crypto-sensitive. Because an inline finding was reported, this note only records the two concerns that were checked and ruled out.

[claude]

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

_{Tip: disable this comment in your organization's Code Review settings.}

[zahin-mohammad]

@claude review

Please review the latest head, 8792aa4, including the signing/key-generation cleanup and the two fixes from Pranav’s review. Both review threads contain implementation and validation details; all 35 focused tests pass locally.

[claude]

I reviewed the latest commits and found no bugs. This changes native session ownership in DKLs ECDSA keygen and signing, and the code is owned by the wallet-core and HSM CODEOWNERS, so a human should still look at it.

What was reviewed:

• Commit footers: all three commits now carry Ticket: WCN-4341. WCN- is a configured issue prefix, so commitlint's references-empty rule should pass.
• dsg.ts: on failure, dsgSessionBytes is no longer re-serialized, so a failed round leaves the previous round's bytes in place. free() errors are swallowed when an earlier error should be kept. In the Complete state the reference is only dropped, because combine consumes the session.
• dkg.ts: the retrofit key share is now freed right after initKeyRotation instead of after round 4. That is safe only if the binding borrows the share rather than keeping it, which the new code comment says but this review could not confirm from the binding itself.

Extended reasoning...

The change touches WASM memory ownership in sdk-lib-mpc's DKLs Dkg and Dsg classes (dkg.ts, dsg.ts) and adds unit tests. This is threshold-signature crypto code, and freeing a session at the wrong time could cause a double free or use-after-free in the native binding. CODEOWNERS assigns /modules/sdk-lib-mpc/ to the wallet-core, wallet-core-india and HSM teams. Both earlier human reviews were dismissed after later commits, so no current human approval exists for the latest code. The earlier commit-footer finding appears fixed by the Ticket footers on the new commits.