diff options
author | Tobias Nießen <tniessen@tnie.de> | 2019-06-16 11:26:03 +0200 |
---|---|---|
committer | Tobias Nießen <tniessen@tnie.de> | 2019-06-18 18:42:48 +0200 |
commit | fc50e6bcc81e4b34f4f3f3fe494b79200ae22efb (patch) | |
tree | 2190309e7b22c5950075affa568a97a6ae90bae8 | |
parent | 8030ca5b9e593a5a800f40f09677a4eca31d1cd0 (diff) | |
download | android-node-v8-fc50e6bcc81e4b34f4f3f3fe494b79200ae22efb.tar.gz android-node-v8-fc50e6bcc81e4b34f4f3f3fe494b79200ae22efb.tar.bz2 android-node-v8-fc50e6bcc81e4b34f4f3f3fe494b79200ae22efb.zip |
crypto: fix crash when calling digest after piping
When piping data into an SHA3 hash, EVP_DigestFinal_ex is called in
hash._flush, bypassing safeguards in the JavaScript layer. Calling
hash.digest causes EVP_DigestFinal_ex to be called again, resulting
in a segmentation fault in the SHA3 implementation of OpenSSL.
A relatively easy solution is to cache the result of calling
EVP_DigestFinal_ex until the Hash object is garbage collected.
PR-URL: https://github.com/nodejs/node/pull/28251
Fixes: https://github.com/nodejs/node/issues/28245
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
-rw-r--r-- | src/node_crypto.cc | 16 | ||||
-rw-r--r-- | src/node_crypto.h | 9 | ||||
-rw-r--r-- | test/parallel/test-crypto-hash-stream-pipe.js | 10 |
3 files changed, 26 insertions, 9 deletions
diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 813e1fc485..590c6d1c37 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -4634,16 +4634,20 @@ void Hash::HashDigest(const FunctionCallbackInfo<Value>& args) { encoding = ParseEncoding(env->isolate(), args[0], BUFFER); } - unsigned char md_value[EVP_MAX_MD_SIZE]; - unsigned int md_len; - - EVP_DigestFinal_ex(hash->mdctx_.get(), md_value, &md_len); + if (hash->md_len_ == 0) { + // Some hash algorithms such as SHA3 do not support calling + // EVP_DigestFinal_ex more than once, however, Hash._flush + // and Hash.digest can both be used to retrieve the digest, + // so we need to cache it. + // See https://github.com/nodejs/node/issues/28245. + EVP_DigestFinal_ex(hash->mdctx_.get(), hash->md_value_, &hash->md_len_); + } Local<Value> error; MaybeLocal<Value> rc = StringBytes::Encode(env->isolate(), - reinterpret_cast<const char*>(md_value), - md_len, + reinterpret_cast<const char*>(hash->md_value_), + hash->md_len_, encoding, &error); if (rc.IsEmpty()) { diff --git a/src/node_crypto.h b/src/node_crypto.h index aa29585533..3e337eaddb 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -595,12 +595,19 @@ class Hash : public BaseObject { Hash(Environment* env, v8::Local<v8::Object> wrap) : BaseObject(env, wrap), - mdctx_(nullptr) { + mdctx_(nullptr), + md_len_(0) { MakeWeak(); } + ~Hash() override { + OPENSSL_cleanse(md_value_, md_len_); + } + private: EVPMDPointer mdctx_; + unsigned char md_value_[EVP_MAX_MD_SIZE]; + unsigned int md_len_; }; class SignBase : public BaseObject { diff --git a/test/parallel/test-crypto-hash-stream-pipe.js b/test/parallel/test-crypto-hash-stream-pipe.js index 0a240a2abb..d22281abbd 100644 --- a/test/parallel/test-crypto-hash-stream-pipe.js +++ b/test/parallel/test-crypto-hash-stream-pipe.js @@ -30,11 +30,17 @@ const crypto = require('crypto'); const stream = require('stream'); const s = new stream.PassThrough(); -const h = crypto.createHash('sha1'); -const expect = '15987e60950cf22655b9323bc1e281f9c4aff47e'; +const h = crypto.createHash('sha3-512'); +const expect = '36a38a2a35e698974d4e5791a3f05b05' + + '198235381e864f91a0e8cd6a26b677ec' + + 'dcde8e2b069bd7355fabd68abd6fc801' + + '19659f25e92f8efc961ee3a7c815c758'; s.pipe(h).on('data', common.mustCall(function(c) { assert.strictEqual(c, expect); + // Calling digest() after piping into a stream with SHA3 should not cause + // a segmentation fault, see https://github.com/nodejs/node/issues/28245. + assert.strictEqual(h.digest('hex'), expect); })).setEncoding('hex'); s.end('aoeu'); |