[robobun]

Problem

• Hmac#digest(encoding) returns 64 bytes of uninitialized stack memory when the toString() of encoding calls digest() on the same Hmac. h.digest({ toString() { h.digest(); return "hex"; } }) returns 128 hex characters that change per call.
• The cause is in jsHmacProtoFuncDigest (src/jsc/bindings/node/crypto/JSHmac.cpp:226-247). The stack buffer length starts at its capacity, and only HMAC_Final lowers it. The inner digest() frees the HMAC context, so the outer call skips HMAC_Final.

Fix

• The encoded length starts at zero. The function sets it only after digestInto, the HMAC_Final wrapper, succeeds.
• The outer call returns "" or an empty Buffer, as digest() does on a finalized Hmac. Node made the same change in crypto: prevent Hmac.digest() from returning uninitialized memory after stream use nodejs/node#65112.
• Verified: test/js/node/crypto/crypto.hmac.test.ts (65 new tests, all fail on a release build of main). Also the suites in Notes.
• Self-reviewed: 10 concerns, all on the scope statement in Notes, now exact. One request is declined (Notes, "Scope").

Background

• Hmac#digest tests m_finalized, then converts a truthy encoding to a string. User code can run there, as in Node.
• Designs weighed: a second m_finalized test after the conversion adds a branch per call. A conversion before the first test calls toString() on a finalized Hmac, unlike Node. A change in ncrypto is not reached on this path.

Downsides

• None found. In release builds of the merge base and the PR, the function runs the same instruction count on five paths. It shrinks by 3 bytes. The stripped binary keeps its size.

Notes

Repro.

import { createHmac } from "node:crypto";
const run = () => {
  const h = createHmac("sha512", "k").update("x");
  const out = h.digest({ toString() { h.digest(); return "hex"; } });
  return out.length + " chars: " + out.slice(0, 32);
};
console.log(run()); console.log(run());
console.log(createHmac("sha512", "k").update("x").digest("hex").slice(0, 32));

Runtime	First two lines	Third line
Release build of main (449877f)	128 chars: 9406dd82b90400000300000001000000, 128 chars: 9406dd82b90400000300000001040000	38fbcb094b99f9a73d862d0cb2b01f98
This PR	0 chars: twice	38fbcb094b99f9a73d862d0cb2b01f98
Node v26.3.0	128 chars: a4e0587e587f0000f9e2fa33701b0000, 128 chars: a4e0587e587f000039d6681bed3f0000	38fbcb094b99f9a73d862d0cb2b01f98
Node v26.12.1 (has nodejs/node#65112)	0 chars: twice	38fbcb094b99f9a73d862d0cb2b01f98

Measurements. The two release builds come from the same HEAD. The merge base build has the JSHmac.cpp hunk reverted in the working tree.

Instructions that jsHmacProtoFuncDigest executes per call. The count is from gdb nexti over the body of the function, with the callees stepped over. Two runs per binary gave the same counts.

Call	Merge base	PR
digest("hex")	163	163
digest()	109	109
digest({ toString })	149	149
digest("hex") on a finalized Hmac	65	65
Outer call of the repro	105	105

• Size of jsHmacProtoFuncDigest (nm -S): 1639 to 1636 bytes. One instruction differs: mov $0x40,%r14d becomes xor %r14d,%r14d. It is in the block that runs only when m_ctx is null.
• size of the stripped binary: text 107556528, data 116336, bss 1819272 in both builds. The file has 107730656 bytes in both builds.
• Call sites in the host function: 29 in both builds, with the same callees.
• GC cells per live digest() call: 1.02 in both builds. The count is from heapStats().objectTypeCounts over 20000 calls with BUN_JSC_useGC=0.
• Host functions in JSHmac.cpp: 4 to 4. Prototype table rows: 2 to 2. Production files changed: 1.
• Bytes that the outer digest("hex") of the repro encodes: 64 to 0. Node v26.12.1: 0.
• BUN_JSC_validateExceptionChecks=1 bun bd test test/js/node/crypto/crypto.hmac.test.ts: 139 pass, 0 reports.

Test matrix. The matrix has 64 rows, and test.each runs each row as one test. The axes: 2 digest algorithms, 4 kinds of conversion hook, 2 ways to finalize in the hook, 4 returned encodings. The 65th test is a nested conversion. Rows in which the outer call returns bytes:

• Release build of main: 64 of 64.
• This PR: 0 of 64.
• This PR with the initial length set back to sizeof(mdValue): 64 of 64.

Suites run on the debug build. test/js/node/crypto/crypto.hmac.test.ts, crypto-lazyhash.test.ts, crypto-extra-memory.test.ts, crypto-invalid-this.test.ts, crypto-hmac-algorithm.test.ts, and from test/js/node/test/parallel/: test-crypto-hmac.js, test-crypto-hash.js, test-crypto-update-encoding.js, test-crypto-hash-stream-pipe.js, test-crypto-lazy-transform-writable.js, test-crypto-stream.js, test-crypto-classes.js.

Failed HMAC_Final. A failed digestInto keeps m_finalized false, as before and as in Node. HMAC_Final does not fail on an initialized context, so no test reaches that path. A later digest() on that path now encodes zero bytes.

Scope. This PR changes Hmac#digest only. No other entry point encodes a capacity-sized length after a check that user code can invalidate. .len = sizeof(...) occurs only in this function, and digestInto has one caller. The other entry points that run user code between a state check and the use of that state fail safe:

• Hash#digest with the same hook returns the cached digest.
• Hash#copy with an outputLength getter that calls digest() throws Digest copy error. For the hashers that do not use EVP_MD_CTX it returns a valid copy.
• Verify#verify with a padding getter that calls verify() throws Failed to finalize digest.
• Sign#sign reads its context after all the option getters.
• Hmac#update and Hash#update read the encoding only when it is a string.

The self-review also asked for a public note and a tracking issue about code that this PR does not touch. I left that to the maintainers.

Not changed here.

• Take an Hmac that is finalized at entry. digest(x) returns an empty Buffer when x is truthy and not a known encoding string. Node returns "". A review comment on this PR has the repro and the cause.
• crypto: Hash, Hmac, Cipheriv, Decipheriv as single native classes; drop LazyTransform #40075 rewrites these classes. Its Hmac digest has the same length guard, but it converts the encoding before the m_finalized test. The new test counts the conversions and fails on that order.

[robobun]

Status

Closed. Main has the fix from #47559 (bc55d56).

• The defect: h.digest({ toString() { h.digest(); return "hex"; } }) on an Hmac returned 128 hex characters of uninitialized stack memory.
• On main's sources the call returns "".
• All 139 tests of this PR's test/js/node/crypto/crypto.hmac.test.ts pass on main's sources. 65 of them are new, and main has no test for this case.

[robobun]

^{Updated 7:51 PM PT - Oct 26th, 2026}

✅ @robobun, your commit 80031a19be1004359339e55585bbf9139c5d5183 passed in Build #133350! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 47026

That installs a local version of the PR into your bun-47026 executable, so you can run:

bun-47026 --bun

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: oven-sh/bun
• Branch: robobun/20e997b8/hmac-digest-unwritten-bytes
• File: .coderabbit.yaml
• Commit: 7eb146f
• Layers: Repository YAML
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 41dc62e3-1e4b-4576-89bd-9f524d9929ce

Commits

Reviewing files that changed from the base of the PR and between 1764634 and 7eb146f.

Files selected for processing: 1

• test/js/node/crypto/crypto.hmac.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

Walkthrough

The HMAC digest function records the length written before encoding the result. Tests cover conversion hooks that finalize the HMAC, nested digest calls, and digest behavior after stream completion.

Changes

HMAC digest finalization

Layer / File(s)	Summary
Digest encoding and finalization src/jsc/bindings/node/crypto/JSHmac.cpp, test/js/node/crypto/crypto.hmac.test.ts	The digest function records the written length and encodes from the saved buffer. Tests check nested finalization during encoding conversion, empty results from outer and subsequent digest calls, finalized-update errors, and digest behavior after stream completion.

Suggested reviewers: alii, jarred-sumner

Priority: Normal

Merge Risk: Minimal · previous review 7eb14

Reentrant digest calls return an empty result without exposing unwritten bytes. No merge-blocking risk was identified.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title clearly identifies the main change: limiting HMAC digest encoding to the bytes written by HMAC_Final.
Description check		The description explains the problem, fix, scope, and verification results in detail. It does not use the template headings exactly, but it provides the required information through equivalent section…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description explains the problem, fix, scope, and verification results in detail. It does not use the template headings exactly, but it provides the required information through equivalent sections.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

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

Beyond the inline findings, I also checked the sibling coerce-then-digest site in Hash#digest (src/jsc/bindings/node/crypto/JSHash.cpp) for the same uninitialized-bytes leak: an encoding hook that finalizes there leaves the digest cached in m_digest, so the outer call serves real bytes rather than a capacity-sized span. The digestInto failure path in the fixed function is unchanged (throws before mdLen is set).

Extended reasoning...

The diff is a small fix in jsHmacProtoFuncDigest that starts the encoded length at zero and sets it only after HMAC_Final succeeds, so an encoding coercion that re-enters digest() yields an empty result instead of 64 uninitialized stack bytes, plus a matrix regression test. It touches node:crypto native bindings, a security-sensitive surface (memory disclosure), which is why a human look remains worthwhile despite the fix itself being minimal and clearly correct. The sibling Hash#digest path was checked and does not share the defect.

Additional findings (outside the current diff — GitHub can't attach inline comments there):

• 🟣 src/jsc/bindings/node/crypto/JSHmac.cpp — Callers that pass a non-string encoding object to digest() on an already-finalized Hmac get an empty Buffer, where Node returns "". The finalized path at src/jsc/bindings/node/crypto/JSHmac.cpp:203 uses parseEnumeration, which returns nullopt for any non-string (src/jsc/bindings/JSBufferEncodingType.cpp:54), so line 206 falls through to the Buffer at 210. The PR fixes the non-finalized sibling of this exact function and its new test even calls digest(encodingArgument) on the finalized Hmac but discards the result. Fix: on the finalized path treat any truthy encoding that is not "buffer" as a string encoding, returning jsEmptyString, and assert that value in the matrix test.

  Why this was flagged

  Trigger: any Hmac whose m_finalized is true (after digest(), end(), or a stream _flush) receives digest(x) where x is truthy and not a string, e.g. the same hook object the new matrix test passes at test/js/node/crypto/crypto.hmac.test.ts:487. JSHmac.cpp:203 calls parseEnumeration, which short-circuits to nullopt for non-strings at JSBufferEncodingType.cpp:54, so the encoding.has_value() test at JSHmac.cpp:206 fails and line 210 returns an empty Buffer. Node's hash.js returns buf.toString(outputEncoding) on a zero-length buffer, which is "" for any truthy encoding. The base branch behaves the same, but the PR rewrites the non-finalized branch of the same function to match Node and the matrix test explicitly exercises this finalized path (line 487) while deliberately not asserting its return value, which is the pattern the review rules call routing a test around a runtime bug. Four finders raised it independently. Remedy: after parseEnumeration returns nullopt for a truthy non-string, return jsEmptyString, matching the base-branch comment's own intent and Node.

  Verification: The trigger is digest(x) on an Hmac whose m_finalized is already true, with x a truthy non-string. src/jsc/bindings/node/crypto/JSHmac.cpp:203 parseEnumeration returns nullopt for any non-string (src/jsc/bindings/JSBufferEncodingType.cpp:54-56), so line 210 returns an empty Buffer where Node returns "". The finalized branch is untouched by the diff, so the base already fails by the same route.

[robobun]

Reply to this review.

Result on an Hmac that is finalized at entry (JSHmac.cpp:196-211). The finding is correct, and it predates this PR. Bun 1.4.2 and a release build of main (449877f) give the same output.

Call on a finalized Hmac	Bun	Node v26.3.0 and v26.12.1
digest(), digest("buffer")	Buffer(0)	Buffer(0)
digest("hex")	""	""
digest("nonsense"), digest("BUFFER"), digest(1), digest({}), digest(Symbol())	Buffer(0)	""

The cause is as the review says. The branch reads the argument with parseEnumeration, which gives no value for a non-string and for an unknown name. Node tests the raw argument, outputEncoding && outputEncoding !== 'buffer' (hash.js#L230-L235), and does not convert it.

This PR does not change that branch. The PR removes the unwritten bytes on the non-finalized path. The finalized branch has a different defect: an empty Buffer in place of an empty string. The diff does not touch those lines, so I kept it out of this change. The description lists it under "Not changed here". No tracking issue exists for it yet. The script below and the cause above are enough to open one. If a maintainer wants the fix in this PR, I can add it. The branch needs the same test of the raw argument that Node has.

The matrix calls digest(encodingArgument) on the finalized Hmac to count the conversions. The count must stay at 1, because Node does not convert the encoding there. The test does not assert the value that this call returns, because this PR leaves that value as it is.

Matrix shape. Done in 7eb146f. The matrix is a test.each table, one test per row.

Script

import { createHmac } from "node:crypto";

const show = v => (typeof v === "string" ? JSON.stringify(v) : `Buffer(${v.length})`);
const finalized = () => {
  const h = createHmac("sha256", "k").update("x");
  h.digest();
  return h;
};

console.log('digest()            ', show(finalized().digest()));
console.log('digest("buffer")    ', show(finalized().digest("buffer")));
console.log('digest("hex")       ', show(finalized().digest("hex")));
console.log('digest("nonsense")  ', show(finalized().digest("nonsense")));
console.log('digest("BUFFER")    ', show(finalized().digest("BUFFER")));
console.log('digest(1)           ', show(finalized().digest(1)));
console.log('digest({})          ', show(finalized().digest({})));
console.log('digest(Symbol())    ', show(finalized().digest(Symbol())));

[claude]

I re-reviewed after the second commit and found no bugs; the matrix now runs one test per row, which addresses my earlier comment. Because this is a native memory-disclosure fix in the node:crypto bindings, a human look is still worthwhile before merging.

What was reviewed:

• The new mdLen flow in jsHmacProtoFuncDigest: HMACCtxPointer::digestInto writes buf->len only on success, so mdLen is 0 on the re-entrant/already-digested path and the failed-HMAC_Final path, and the encode span never covers unwritten mdValue bytes.
• Checked jsHashProtoFuncDigest for the same coerce-then-digest shape: it serves re-entrant calls from the cached m_digest, so it is not affected.
• Checked the test's "second digest(encodingArgument) does not re-run the hook" assertion against the finalized branch: parseEnumeration<BufferEncodingType> returns nullopt for non-strings without coercing, so the count stays 1.

Extended reasoning...

The change touches two files: a ~10-line edit in src/jsc/bindings/node/crypto/JSHmac.cpp that initializes the encoded length to zero and sets it only after HMAC_Final succeeds, and a new 64-row test.each matrix plus a nested-conversion test in test/js/node/crypto/crypto.hmac.test.ts. It fixes an uninitialized-stack disclosure reachable when the encoding argument's toString/valueOf/Symbol.toPrimitive/Proxy trap re-enters digest() or end(), and cites the matching upstream Node change. The second commit converted the matrix from one looped test to test.each, resolving the only prior inline comment. No CODEOWNERS entry covers the changed paths and no findings were produced, but the change is a memory-safety fix inside the node:crypto native bindings, which is the deciding factor for deferring rather than approving outright.

[robobun]

Closing: #47559 landed this fix on main (bc55d56). It adds an else branch to the same block of jsHmacProtoFuncDigest. Hmac#digest(encoding) now returns an empty digest when the conversion of encoding finalizes the Hmac.

I ran this PR's test file against main's sources. All 139 tests of test/js/node/crypto/crypto.hmac.test.ts pass. The repro from the description returns a string of length 0.

Main has no test for this case. The 65 new tests are in this branch if a maintainer wants them as a test-only change.

The difference on an Hmac that is finalized at entry is still on main. This comment has the repro and the cause.