From 9ec87815027ddf6782ab930975b86333a61ed554 Mon Sep 17 00:00:00 2001
From: Ben Noordhuis <info@bnoordhuis.nl>
Date: Thu, 13 Feb 2020 12:05:18 +0100
Subject: [PATCH] crypto: make update(buf, enc) ignore encoding
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Make the cipher/decipher/hash/hmac update() methods ignore the input
encoding when the input is a buffer.

This is the documented behavior but some inputs were rejected, notably
when the specified encoding is 'hex' and the buffer has an odd length
(because a _string_ with an odd length is never a valid hex string.)

The sign/verify update() methods work okay because they use different
validation logic.

Fixes: https://github.com/nodejs/node/issues/31751

PR-URL: https://github.com/nodejs/node/pull/31766
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
---
 lib/internal/crypto/cipher.js                |  6 +++---
 lib/internal/crypto/hash.js                  | 14 +++++--------
 test/parallel/test-crypto-update-encoding.js | 22 ++++++++++++++++++++
 3 files changed, 30 insertions(+), 12 deletions(-)
 create mode 100644 test/parallel/test-crypto-update-encoding.js

diff --git a/lib/internal/crypto/cipher.js b/lib/internal/crypto/cipher.js
index add56eae680ced..80b0c0e9dab382 100644
--- a/lib/internal/crypto/cipher.js
+++ b/lib/internal/crypto/cipher.js
@@ -151,13 +151,13 @@ Cipher.prototype.update = function update(data, inputEncoding, outputEncoding) {
   inputEncoding = inputEncoding || encoding;
   outputEncoding = outputEncoding || encoding;
 
-  if (typeof data !== 'string' && !isArrayBufferView(data)) {
+  if (typeof data === 'string') {
+    validateEncoding(data, inputEncoding);
+  } else if (!isArrayBufferView(data)) {
     throw new ERR_INVALID_ARG_TYPE(
       'data', ['string', 'Buffer', 'TypedArray', 'DataView'], data);
   }
 
-  validateEncoding(data, inputEncoding);
-
   const ret = this[kHandle].update(data, inputEncoding);
 
   if (outputEncoding && outputEncoding !== 'buffer') {
diff --git a/lib/internal/crypto/hash.js b/lib/internal/crypto/hash.js
index dca0ba767f6e29..1cf0188da2f35e 100644
--- a/lib/internal/crypto/hash.js
+++ b/lib/internal/crypto/hash.js
@@ -78,17 +78,13 @@ Hash.prototype.update = function update(data, encoding) {
   if (state[kFinalized])
     throw new ERR_CRYPTO_HASH_FINALIZED();
 
-  if (typeof data !== 'string' && !isArrayBufferView(data)) {
-    throw new ERR_INVALID_ARG_TYPE('data',
-                                   ['string',
-                                    'Buffer',
-                                    'TypedArray',
-                                    'DataView'],
-                                   data);
+  if (typeof data === 'string') {
+    validateEncoding(data, encoding);
+  } else if (!isArrayBufferView(data)) {
+    throw new ERR_INVALID_ARG_TYPE(
+      'data', ['string', 'Buffer', 'TypedArray', 'DataView'], data);
   }
 
-  validateEncoding(data, encoding);
-
   if (!this[kHandle].update(data, encoding))
     throw new ERR_CRYPTO_HASH_UPDATE_FAILED();
   return this;
diff --git a/test/parallel/test-crypto-update-encoding.js b/test/parallel/test-crypto-update-encoding.js
new file mode 100644
index 00000000000000..e1e6d029aa5e30
--- /dev/null
+++ b/test/parallel/test-crypto-update-encoding.js
@@ -0,0 +1,22 @@
+'use strict';
+const common = require('../common');
+
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+const crypto = require('crypto');
+
+const zeros = Buffer.alloc;
+const key = zeros(16);
+const iv = zeros(16);
+
+const cipher = () => crypto.createCipheriv('aes-128-cbc', key, iv);
+const decipher = () => crypto.createDecipheriv('aes-128-cbc', key, iv);
+const hash = () => crypto.createSign('sha256');
+const hmac = () => crypto.createHmac('sha256', key);
+const sign = () => crypto.createSign('sha256');
+const verify = () => crypto.createVerify('sha256');
+
+for (const f of [cipher, decipher, hash, hmac, sign, verify])
+  for (const n of [15, 16])
+    f().update(zeros(n), 'hex');  // Should ignore inputEncoding.