From 814f4f1314bcb0d32d9b70469a58f288f00b4a22 Mon Sep 17 00:00:00 2001
From: Ben Noordhuis <info@bnoordhuis.nl>
Date: Mon, 10 Jul 2017 12:56:37 +0200
Subject: [PATCH] src: avoid heap allocation in hmac.digest()

Add a test that ensures the second call to .digest() returns an empty
HMAC, like it did before.  No comment on whether that is the right
behavior or not.

PR-URL: https://github.com/nodejs/node/pull/14122
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 src/node_crypto.cc                | 22 +++++--------------
 src/node_crypto.h                 |  1 -
 test/parallel/test-crypto-hmac.js | 35 +++++++++++++++++++++++++++++++
 3 files changed, 40 insertions(+), 18 deletions(-)

diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index a4f38595916281..573688e2511248 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -3767,17 +3767,6 @@ void Hmac::HmacUpdate(const FunctionCallbackInfo<Value>& args) {
 }
 
 
-bool Hmac::HmacDigest(unsigned char** md_value, unsigned int* md_len) {
-  if (!initialised_)
-    return false;
-  *md_value = new unsigned char[EVP_MAX_MD_SIZE];
-  HMAC_Final(&ctx_, *md_value, md_len);
-  HMAC_CTX_cleanup(&ctx_);
-  initialised_ = false;
-  return true;
-}
-
-
 void Hmac::HmacDigest(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
@@ -3794,13 +3783,13 @@ void Hmac::HmacDigest(const FunctionCallbackInfo<Value>& args) {
     return env->ThrowError("hmac.digest() does not support UTF-16");
   }
 
-  unsigned char* md_value = nullptr;
+  unsigned char md_value[EVP_MAX_MD_SIZE];
   unsigned int md_len = 0;
 
-  bool r = hmac->HmacDigest(&md_value, &md_len);
-  if (!r) {
-    md_value = nullptr;
-    md_len = 0;
+  if (hmac->initialised_) {
+    HMAC_Final(&hmac->ctx_, md_value, &md_len);
+    HMAC_CTX_cleanup(&hmac->ctx_);
+    hmac->initialised_ = false;
   }
 
   Local<Value> error;
@@ -3810,7 +3799,6 @@ void Hmac::HmacDigest(const FunctionCallbackInfo<Value>& args) {
                           md_len,
                           encoding,
                           &error);
-  delete[] md_value;
   if (rc.IsEmpty()) {
     CHECK(!error.IsEmpty());
     env->isolate()->ThrowException(error);
diff --git a/src/node_crypto.h b/src/node_crypto.h
index a2b8cb7a8872eb..eb5ca2b770b2c8 100644
--- a/src/node_crypto.h
+++ b/src/node_crypto.h
@@ -494,7 +494,6 @@ class Hmac : public BaseObject {
  protected:
   void HmacInit(const char* hash_type, const char* key, int key_len);
   bool HmacUpdate(const char* data, int len);
-  bool HmacDigest(unsigned char** md_value, unsigned int* md_len);
 
   static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void HmacInit(const v8::FunctionCallbackInfo<v8::Value>& args);
diff --git a/test/parallel/test-crypto-hmac.js b/test/parallel/test-crypto-hmac.js
index a10e8a731be023..6ddffbf56f23b2 100644
--- a/test/parallel/test-crypto-hmac.js
+++ b/test/parallel/test-crypto-hmac.js
@@ -379,3 +379,38 @@ for (let i = 0, l = rfc2202_sha1.length; i < l; i++) {
 assert.throws(function() {
   crypto.createHmac('sha256', 'w00t').digest('ucs2');
 }, /^Error: hmac\.digest\(\) does not support UTF-16$/);
+
+// 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.deepStrictEqual(h.digest('latin1'), expected);
+    assert.deepStrictEqual(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.deepStrictEqual(h.digest('latin1'), expected);
+    assert.deepStrictEqual(h.digest('latin1'), '');
+  }
+}