fix(crypto): erase intermediate password bytes in v1 KDF input builder - #162
Merged
Merged
Conversation
buildBaseMaterial encoded the NFC password into passwordBytes and, in the no-key-file path, copied it into a second buffer that the caller erases, leaving the original passwordBytes copy unzeroed until garbage collection. Return the encoded bytes directly so the caller's secureErase lands on the one copy that exists. In the key-file path, zero passwordBytes after it is concatenated into the returned material. Byte-neutral: the KDF input is unchanged, so derived keys and container bytes are identical. Verified by test:crypto (45 passed) and test:keymaker (238 passed).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
buildBaseMaterial(src/lib/keymaker-crypto.ts) builds the v1 KDF input from the NFC-normalized password and an optional key file. It left a second, unzeroed copy of the password in memory:passwordBytes, then copied those bytes into a fresh buffer and returned the copy. The caller erases the returned buffer, but the originalpasswordByteswas never zeroed, so a second copy of the password sat in the heap until GC.passwordByteswas concatenated into the returned material and then left unzeroed.Change
passwordBytesdirectly. It is already a fresh, exact-lengthUint8Array, so the copy was redundant, and returning it means the caller'ssecureErase(baseMaterial)lands on the one copy that exists.secureErase(passwordBytes)after it is copied into the returned buffer.This closes the only memory-hygiene gap that survived verification of a broader review. The reported v2/worker "key-file leak", "main-thread leak", and "worker
finally" findings were checked against the source and are not real:secureErasezeroes the backingArrayBuffer, and every v2 path passes a view over the same key-file buffer that the caller'sfinallyerases, so no separate copy lingers.Why it is safe
Byte-neutral. The KDF input is identical, so derived keys and container bytes do not change. The v1 encrypt and decrypt paths both already erase the returned
baseMaterial.Tests
npm run typecheck: clean.npm run test:crypto: 45 passed, 0 failed.npm run test:keymaker: 238 passed, 0 failed.The change is byte-neutral, so the existing regression and conformance suites are the correctness control; there is no new externally observable behavior to assert.
🤖 Generated with Claude Code
Generated by Claude Code