Skip to content

crypto: prevent Hmac.digest() from returning uninitialized memory after stream use - #65112

Open
mcollina wants to merge 1 commit into
nodejs:mainfrom
mcollina:fix-hmac-stream-digest-uninitialized-memory
Open

crypto: prevent Hmac.digest() from returning uninitialized memory after stream use#65112
mcollina wants to merge 1 commit into
nodejs:mainfrom
mcollina:fix-hmac-stream-digest-uninitialized-memory

Conversation

@mcollina

@mcollina mcollina commented Aug 7, 2026

Copy link
Copy Markdown
Member

Hmac.prototype._flush was aliased to Hash.prototype._flush, which finalizes the native HMAC context but never sets the JavaScript-side kFinalized flag. After an Hmac has been used as a stream, a subsequent Hmac.prototype.digest() call therefore still believes the object has not been finalized and calls into C++ a second time. On that second call the native context has already been reset, so the digest buffer is never written and Digest::MAX_SIZE bytes of uninitialized stack memory are returned to JavaScript.

Hash is not affected because Hash::HashDigest caches its digest (refs #28245); Hmac never received the equivalent protection.

This gives Hmac its own _flush that sets kFinalized, so repeat digest() calls after stream use are handled by the existing DEP0206 guard. As defense in depth, it also sets buf.len = 0 on the native side when the context has already been reset, so unwritten bytes can never be emitted. A regression test covers the stream-then-digest() path.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/crypto

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. needs-ci PRs that need a full CI run. labels Aug 7, 2026
@codecov

codecov Bot commented Aug 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.13%. Comparing base (e6a8d06) to head (e41218a).
⚠️ Report is 821 commits behind head on main.

Files with missing lines Patch % Lines
src/crypto/crypto_hmac.cc 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #65112      +/-   ##
==========================================
- Coverage   92.01%   90.13%   -1.88%     
==========================================
  Files         379      752     +373     
  Lines      166972   251826   +84854     
  Branches    25554    47354   +21800     
==========================================
+ Hits       153639   226984   +73345     
- Misses      13041    16177    +3136     
- Partials      292     8665    +8373     
Files with missing lines Coverage Δ
lib/internal/crypto/hash.js 98.17% <100.00%> (-0.86%) ⬇️
src/crypto/crypto_hmac.cc 68.71% <50.00%> (ø)

... and 600 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Hmac.prototype._flush was aliased to Hash.prototype._flush, which
finalizes the native HMAC context but never sets the JavaScript-side
kFinalized flag. After an Hmac has been used as a stream, a subsequent
Hmac.prototype.digest() call therefore still believes the object has not
been finalized and calls into C++ a second time. On that second call the
native context has already been reset, so the digest buffer is never
written and Digest::MAX_SIZE bytes of uninitialized stack memory are
returned to JavaScript.

Hash is not affected because Hash::HashDigest caches its digest
(refs nodejs#28245); Hmac never received the equivalent protection.

Give Hmac its own _flush that sets kFinalized so repeat digest() calls
after stream use are handled by the existing DEP0206 guard. As defense
in depth, also set buf.len = 0 on the native side when the context has
already been reset so unwritten bytes can never be emitted.

Signed-off-by: Matteo Collina <hello@matteocollina.com>
@mcollina
mcollina force-pushed the fix-hmac-stream-digest-uninitialized-memory branch from e410836 to e41218a Compare August 17, 2026 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants