From ea8d83bf596c1285ed6288281f6fb1bb0f0a6f75 Mon Sep 17 00:00:00 2001
From: XadillaX <i@2333.moe>
Date: Thu, 3 Jun 2021 14:27:32 +0800
Subject: [PATCH] src,crypto: fix 0-length output crash in webcrypto
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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

PR-URL: https://github.com/nodejs/node/pull/38913
Refs: https://github.com/nodejs/node/issues/38883
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
---
 src/crypto/crypto_aes.cc                      | 12 +++++-
 src/crypto/crypto_cipher.h                    |  9 +++--
 .../test-crypto-subtle-zero-length.js         | 39 +++++++++++++++++++
 3 files changed, 55 insertions(+), 5 deletions(-)
 create mode 100644 test/parallel/test-crypto-subtle-zero-length.js

diff --git a/src/crypto/crypto_aes.cc b/src/crypto/crypto_aes.cc
index edca8fbd0dcb5c..d6f6393df6c878 100644
--- a/src/crypto/crypto_aes.cc
+++ b/src/crypto/crypto_aes.cc
@@ -130,7 +130,17 @@ WebCryptoCipherStatus AES_Cipher(
   ByteSource buf = ByteSource::Allocated(data, buf_len);
   unsigned char* ptr = reinterpret_cast<unsigned char*>(data);
 
-  if (!EVP_CipherUpdate(
+  // In some outdated version of OpenSSL (e.g.
+  // ubi81_sharedlibs_openssl111fips_x64) may be used in sharedlib mode, the
+  // logic will be failed when input size is zero. The newly OpenSSL has fixed
+  // it up. But we still have to regard zero as special in Node.js code to
+  // prevent old OpenSSL failure.
+  //
+  // Refs: https://github.com/openssl/openssl/commit/420cb707b880e4fb649094241371701013eeb15f
+  // Refs: https://github.com/nodejs/node/pull/38913#issuecomment-866505244
+  if (in.size() == 0) {
+    out_len = 0;
+  } else if (!EVP_CipherUpdate(
           ctx.get(),
           ptr,
           &out_len,
diff --git a/src/crypto/crypto_cipher.h b/src/crypto/crypto_cipher.h
index c8dd3e48f718fd..b9b850a1d64c8b 100644
--- a/src/crypto/crypto_cipher.h
+++ b/src/crypto/crypto_cipher.h
@@ -249,16 +249,17 @@ class CipherJob final : public CryptoJob<CipherTraits> {
       v8::Local<v8::Value>* result) override {
     Environment* env = AsyncWrap::env();
     CryptoErrorStore* errors = CryptoJob<CipherTraits>::errors();
-    if (out_.size() > 0) {
+
+    if (errors->Empty())
+      errors->Capture();
+
+    if (out_.size() > 0 || errors->Empty()) {
       CHECK(errors->Empty());
       *err = v8::Undefined(env->isolate());
       *result = out_.ToArrayBuffer(env);
       return v8::Just(!result->IsEmpty());
     }
 
-    if (errors->Empty())
-      errors->Capture();
-    CHECK(!errors->Empty());
     *result = v8::Undefined(env->isolate());
     return v8::Just(errors->ToException(env).ToLocal(err));
   }
diff --git a/test/parallel/test-crypto-subtle-zero-length.js b/test/parallel/test-crypto-subtle-zero-length.js
new file mode 100644
index 00000000000000..ffca84cf56129e
--- /dev/null
+++ b/test/parallel/test-crypto-subtle-zero-length.js
@@ -0,0 +1,39 @@
+'use strict';
+
+const common = require('../common');
+
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+const assert = require('assert');
+const crypto = require('crypto').webcrypto;
+
+(async () => {
+  const k = await crypto.subtle.importKey(
+    'raw',
+    new Uint8Array(32),
+    { name: 'AES-GCM' },
+    false,
+    [ 'encrypt', 'decrypt' ]);
+  assert(k instanceof crypto.CryptoKey);
+
+  const e = await crypto.subtle.encrypt({
+    name: 'AES-GCM',
+    iv: new Uint8Array(12),
+  }, k, new Uint8Array(0));
+  assert(e instanceof ArrayBuffer);
+  assert.deepStrictEqual(
+    Buffer.from(e),
+    Buffer.from([
+      0x53, 0x0f, 0x8a, 0xfb, 0xc7, 0x45, 0x36, 0xb9,
+      0xa9, 0x63, 0xb4, 0xf1, 0xc4, 0xcb, 0x73, 0x8b ]));
+
+  const v = await crypto.subtle.decrypt({
+    name: 'AES-GCM',
+    iv: new Uint8Array(12),
+  }, k, e);
+  assert(v instanceof ArrayBuffer);
+  assert.strictEqual(v.byteLength, 0);
+})().then(common.mustCall()).catch((e) => {
+  assert.ifError(e);
+});