From e4108363b1c4763ba338c6906730a1ba9ef317c4 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Fri, 7 Aug 2026 15:06:25 +0000 Subject: [PATCH 1/2] crypto: prevent Hmac.digest() from returning uninitialized memory 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. 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. --- lib/internal/crypto/hash.js | 6 +++++- src/crypto/crypto_hmac.cc | 3 +++ test/parallel/test-crypto-hmac.js | 20 ++++++++++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/lib/internal/crypto/hash.js b/lib/internal/crypto/hash.js index 16834f169a5b..b879d467b0c6 100644 --- a/lib/internal/crypto/hash.js +++ b/lib/internal/crypto/hash.js @@ -187,7 +187,11 @@ Hmac.prototype.digest = function digest(outputEncoding) { return ret; }; -Hmac.prototype._flush = Hash.prototype._flush; +Hmac.prototype._flush = function _flush(callback) { + this.push(this[kHandle].digest()); + this[kState][kFinalized] = true; + callback(); +}; Hmac.prototype._transform = Hash.prototype._transform; // Implementation for WebCrypto subtle.digest() diff --git a/src/crypto/crypto_hmac.cc b/src/crypto/crypto_hmac.cc index 42f3b53da0ea..c8328f8ec4fd 100644 --- a/src/crypto/crypto_hmac.cc +++ b/src/crypto/crypto_hmac.cc @@ -141,6 +141,9 @@ void Hmac::HmacDigest(const FunctionCallbackInfo& args) { return ThrowCryptoError(env, ERR_get_error(), "Failed to finalize HMAC"); } hmac->ctx_.reset(); + } else { + // The context has already been finalized; never emit unwritten bytes. + buf.len = 0; } Local ret; diff --git a/test/parallel/test-crypto-hmac.js b/test/parallel/test-crypto-hmac.js index 9ddc4a4b880f..d5c16d9aa834 100644 --- a/test/parallel/test-crypto-hmac.js +++ b/test/parallel/test-crypto-hmac.js @@ -296,6 +296,26 @@ for (let i = 0, l = rfc4231.length; i < l; i++) { } } +// Calling digest() after the Hmac has already been used as a stream must +// return an empty buffer (the DEP0206 repeat-digest guard), not uninitialized +// stack memory. The stream itself must still produce the correct digest. +// See: https://github.com/nodejs/node/issues/28245 +{ + const key = 'key'; + const data = 'some data to hash'; + + const streamHmac = crypto.createHmac('sha256', key); + streamHmac.end(data); + const streamDigest = streamHmac.read(); + + // digest() after the stream already finalized must not return garbage. + assert.deepStrictEqual(streamHmac.digest(), Buffer.from('')); + + // Sanity check: the stream itself produced the correct digest. + const expected = crypto.createHmac('sha256', key).update(data).digest(); + assert.deepStrictEqual(streamDigest, expected); +} + // Test HMAC-MD5/SHA1 (rfc 2202 Test Cases) const rfc2202_md5 = [ { From c19fc3b11d166dba6199429622c0cfe1d310d2d2 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sat, 8 Aug 2026 14:23:54 +0000 Subject: [PATCH 2/2] crypto: make DEP0206 end-of-life DEP0206 (calling Hmac.digest() more than once) is a runtime deprecation that returns an empty buffer instead of throwing, which is inconsistent with hash.digest() and can mask misuse. Make it end-of-life by throwing ERR_CRYPTO_HASH_FINALIZED on a finalized Hmac instance, matching the behavior of Hash.digest(). This also closes the stream path: with the kFinalized flag now set by Hmac.prototype._flush, digest() after the Hmac has been used as a stream throws instead of reaching the native side and returning uninitialized stack memory. Remove the now-unused DEP0206 warning emitter and the empty-buffer return path, and update the tests to verify the throw on both the direct and stream paths. --- lib/internal/crypto/hash.js | 19 +------ test/parallel/test-crypto-dep0206.js | 56 --------------------- test/parallel/test-crypto-hmac-finalized.js | 34 +++++++++++++ test/parallel/test-crypto-hmac.js | 7 +-- 4 files changed, 40 insertions(+), 76 deletions(-) delete mode 100644 test/parallel/test-crypto-dep0206.js create mode 100644 test/parallel/test-crypto-hmac-finalized.js diff --git a/lib/internal/crypto/hash.js b/lib/internal/crypto/hash.js index b879d467b0c6..7f4c8f0df872 100644 --- a/lib/internal/crypto/hash.js +++ b/lib/internal/crypto/hash.js @@ -40,13 +40,8 @@ const { lazyDOMException, normalizeEncoding, encodingsMap, - getDeprecationWarningEmitter, } = require('internal/util'); -const { - Buffer, -} = require('buffer'); - const { codes: { ERR_CRYPTO_HASH_FINALIZED, @@ -72,11 +67,6 @@ const LazyTransform = require('internal/streams/lazy_transform'); const kState = Symbol('kState'); const kFinalized = Symbol('kFinalized'); -const emitHmacDigestDeprecation = getDeprecationWarningEmitter( - 'DEP0206', - 'Calling Hmac.digest() more than once is deprecated.', -); - function Hash(algorithm, options) { if (!new.target) return new Hash(algorithm, options); @@ -173,13 +163,8 @@ Hmac.prototype.update = Hash.prototype.update; Hmac.prototype.digest = function digest(outputEncoding) { const state = this[kState]; - if (state[kFinalized]) { - emitHmacDigestDeprecation(); - const buf = Buffer.from(''); - if (outputEncoding && outputEncoding !== 'buffer') - return buf.toString(outputEncoding); - return buf; - } + if (state[kFinalized]) + throw new ERR_CRYPTO_HASH_FINALIZED(); // Explicit conversion of truthy values for backward compatibility. const ret = this[kHandle].digest(outputEncoding && `${outputEncoding}`); diff --git a/test/parallel/test-crypto-dep0206.js b/test/parallel/test-crypto-dep0206.js deleted file mode 100644 index 4c48ddad6f9e..000000000000 --- a/test/parallel/test-crypto-dep0206.js +++ /dev/null @@ -1,56 +0,0 @@ -'use strict'; - -const common = require('../common'); -if (!common.hasCrypto) - common.skip('missing crypto'); - -const assert = require('assert'); -const crypto = require('crypto'); - -common.expectWarning({ - DeprecationWarning: { - DEP0206: 'Calling Hmac.digest() more than once is deprecated.', - }, -}); - -// Verify runtime deprecation warning for calling digest() more than once. -{ - const h = crypto.createHmac('sha1', 'key').update('data'); - h.digest('hex'); - h.digest('hex'); -} - -// Check initialized -> uninitialized state transition after calling digest(). -{ - const expected = - '\u0010\u0041\u0052\u00c5\u00bf\u00dc\u00a0\u007b\u00c6\u0033' + - '\u00ee\u00bd\u0046\u0019\u009f\u0002\u0055\u00c9\u00f4\u009d'; - { - const h = crypto.createHmac('sha1', 'key').update('data'); - assert.deepStrictEqual(h.digest('buffer'), Buffer.from(expected, 'latin1')); - assert.deepStrictEqual(h.digest('buffer'), Buffer.from('')); - } - { - const h = crypto.createHmac('sha1', 'key').update('data'); - assert.strictEqual(h.digest('latin1'), expected); - assert.strictEqual(h.digest('latin1'), ''); - } -} - -// Check initialized -> uninitialized state transition after calling digest(). -// Calls to update() omitted intentionally. -{ - const expected = - '\u00f4\u002b\u00b0\u00ee\u00b0\u0018\u00eb\u00bd\u0045\u0097' + - '\u00ae\u0072\u0013\u0071\u001e\u00c6\u0007\u0060\u0084\u003f'; - { - const h = crypto.createHmac('sha1', 'key'); - assert.deepStrictEqual(h.digest('buffer'), Buffer.from(expected, 'latin1')); - assert.deepStrictEqual(h.digest('buffer'), Buffer.from('')); - } - { - const h = crypto.createHmac('sha1', 'key'); - assert.strictEqual(h.digest('latin1'), expected); - assert.strictEqual(h.digest('latin1'), ''); - } -} diff --git a/test/parallel/test-crypto-hmac-finalized.js b/test/parallel/test-crypto-hmac-finalized.js new file mode 100644 index 000000000000..51af3bf400e9 --- /dev/null +++ b/test/parallel/test-crypto-hmac-finalized.js @@ -0,0 +1,34 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('assert'); +const crypto = require('crypto'); + +// DEP0206 reached end-of-life: calling Hmac.digest() on a finalized instance +// now throws ERR_CRYPTO_HASH_FINALIZED instead of returning an empty buffer. +// See: https://github.com/nodejs/node/pull/65112 + +// Repeated digest() on the same instance throws. +{ + const h = crypto.createHmac('sha1', 'key').update('data'); + assert.strictEqual(h.digest('hex').length, 40); + assert.throws(() => h.digest('hex'), { code: 'ERR_CRYPTO_HASH_FINALIZED' }); +} + +// digest() with no data throws on a second call. +{ + const h = crypto.createHmac('sha1', 'key'); + assert.strictEqual(h.digest('buffer').length, 20); + assert.throws(() => h.digest('buffer'), { code: 'ERR_CRYPTO_HASH_FINALIZED' }); +} + +// digest() after the Hmac has been used as a stream throws. +{ + const h = crypto.createHmac('sha1', 'key'); + h.end('data'); + assert.strictEqual(h.read().length, 20); + assert.throws(() => h.digest('buffer'), { code: 'ERR_CRYPTO_HASH_FINALIZED' }); +} diff --git a/test/parallel/test-crypto-hmac.js b/test/parallel/test-crypto-hmac.js index d5c16d9aa834..b5a0a2419b43 100644 --- a/test/parallel/test-crypto-hmac.js +++ b/test/parallel/test-crypto-hmac.js @@ -297,8 +297,9 @@ for (let i = 0, l = rfc4231.length; i < l; i++) { } // Calling digest() after the Hmac has already been used as a stream must -// return an empty buffer (the DEP0206 repeat-digest guard), not uninitialized -// stack memory. The stream itself must still produce the correct digest. +// throw ERR_CRYPTO_HASH_FINALIZED (the DEP0206 end-of-life behavior), not +// return uninitialized stack memory. The stream itself must still produce the +// correct digest. // See: https://github.com/nodejs/node/issues/28245 { const key = 'key'; @@ -309,7 +310,7 @@ for (let i = 0, l = rfc4231.length; i < l; i++) { const streamDigest = streamHmac.read(); // digest() after the stream already finalized must not return garbage. - assert.deepStrictEqual(streamHmac.digest(), Buffer.from('')); + assert.throws(() => streamHmac.digest(), { code: 'ERR_CRYPTO_HASH_FINALIZED' }); // Sanity check: the stream itself produced the correct digest. const expected = crypto.createHmac('sha256', key).update(data).digest();