From ab862e8f4e4a96fd4f8618b4baefc5233690e1fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Fri, 28 Dec 2018 16:07:28 +0100 Subject: [PATCH 1/2] crypto: fix zero byte allocation assertion failure When an empty string was passed, malloc might have returned a nullptr depending on the platform, causing an assertion failure. This change makes private key parsing behave as public key parsing does, causing a BIO error instead that can be caught in JS. Fixes: https://github.com/nodejs/node/issues/25247 --- src/node_crypto.cc | 8 +++++--- test/parallel/test-crypto-key-objects.js | 7 +++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index c2faad0a5966bc..23a9e313331b7b 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -2699,7 +2699,7 @@ static bool IsSupportedAuthenticatedMode(const EVP_CIPHER_CTX* ctx) { template static T* MallocOpenSSL(size_t count) { void* mem = OPENSSL_malloc(MultiplyWithOverflowCheck(count, sizeof(T))); - CHECK_NOT_NULL(mem); + CHECK_IMPLIES(mem == nullptr, count == 0); return static_cast(mem); } @@ -2857,7 +2857,8 @@ static EVPKeyPointer ParsePrivateKey(const PrivateKeyEncodingConfig& config, if (config.format_ == kKeyFormatPEM) { BIOPointer bio(BIO_new_mem_buf(key, key_len)); - CHECK(bio); + if (!bio) + return pkey; char* pass = const_cast(config.passphrase_.get()); pkey.reset(PEM_read_bio_PrivateKey(bio.get(), @@ -2872,7 +2873,8 @@ static EVPKeyPointer ParsePrivateKey(const PrivateKeyEncodingConfig& config, pkey.reset(d2i_PrivateKey(EVP_PKEY_RSA, nullptr, &p, key_len)); } else if (config.type_.ToChecked() == kKeyEncodingPKCS8) { BIOPointer bio(BIO_new_mem_buf(key, key_len)); - CHECK(bio); + if (!bio) + return pkey; char* pass = const_cast(config.passphrase_.get()); pkey.reset(d2i_PKCS8PrivateKey_bio(bio.get(), nullptr, diff --git a/test/parallel/test-crypto-key-objects.js b/test/parallel/test-crypto-key-objects.js index dddbd5f2703d93..bdefe7a63513e8 100644 --- a/test/parallel/test-crypto-key-objects.js +++ b/test/parallel/test-crypto-key-objects.js @@ -105,3 +105,10 @@ const privatePem = fixtures.readSync('test_rsa_privkey.pem', 'ascii'); } } } + +{ + // This should not cause a crash: https://github.com/nodejs/node/issues/25247 + assert.throws(() => { + createPrivateKey({ key: '' }); + }); +} From 3c5b00a24de5753ab74c788d28b42c902c859876 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Fri, 28 Dec 2018 16:32:54 +0100 Subject: [PATCH 2/2] fixup! crypto: fix zero byte allocation assertion failure --- test/parallel/test-crypto-key-objects.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-crypto-key-objects.js b/test/parallel/test-crypto-key-objects.js index bdefe7a63513e8..8a28ac996084a5 100644 --- a/test/parallel/test-crypto-key-objects.js +++ b/test/parallel/test-crypto-key-objects.js @@ -110,5 +110,5 @@ const privatePem = fixtures.readSync('test_rsa_privkey.pem', 'ascii'); // This should not cause a crash: https://github.com/nodejs/node/issues/25247 assert.throws(() => { createPrivateKey({ key: '' }); - }); + }, /null/); }