crypto: prevent Hmac.digest() from returning uninitialized memory · nodejs/node@98ca5e9 · GitHub
Skip to content

Commit 98ca5e9

Browse files
mcollinaaduh95
authored andcommitted
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. Signed-off-by: Matteo Collina <hello@matteocollina.com> PR-URL: #65112 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: Filip Skokan <panva.ip@gmail.com>
1 parent a1ae3dd commit 98ca5e9

3 files changed

Lines changed: 29 additions & 1 deletion

File tree

‎lib/internal/crypto/hash.js‎

Lines changed: 6 additions & 1 deletion

‎src/crypto/crypto_hmac.cc‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,9 @@ void Hmac::HmacDigest(const FunctionCallbackInfo<Value>& args) {
141141
return ThrowCryptoError(env, ERR_get_error(), "Failed to finalize HMAC");
142142
}
143143
hmac->ctx_.reset();
144+
} else {
145+
// The context has already been finalized; never emit unwritten bytes.
146+
buf.len = 0;
144147
}
145148

146149
Local<Value> ret;

‎test/parallel/test-crypto-hmac.js‎

Lines changed: 20 additions & 0 deletions

0 commit comments

Comments
 (0)