src,crypto: fix 0-length output crash in webcrypto

Fixes: #38883 PR-URL: #38913 Refs: #38883 Reviewed-By: Tobias Nießen <[email protected]>
nodejs · Jul 11, 2021 · ea8d83b · ea8d83b
1 parent 90ec766
commit ea8d83b
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 5 deletions.
diff --git 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
@@ -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
@@ -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);
+});