Skip to content

Commit

Permalink
src,crypto: fix 0-length output crash in webcrypto
Browse files Browse the repository at this point in the history
Fixes: #38883

PR-URL: #38913
Refs: #38883
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
  • Loading branch information
XadillaX authored and targos committed Jul 11, 2021
1 parent 90ec766 commit ea8d83b
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 5 deletions.
12 changes: 11 additions & 1 deletion src/crypto/crypto_aes.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
9 changes: 5 additions & 4 deletions src/crypto/crypto_cipher.h
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down
39 changes: 39 additions & 0 deletions test/parallel/test-crypto-subtle-zero-length.js
Original file line number Diff line number Diff line change
@@ -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);
});

0 comments on commit ea8d83b

Please sign in to comment.