Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 990feaf

Browse filesBrowse files
tniessentargos
authored andcommitted
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: #28251 Fixes: #28245 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
1 parent 17efd93 commit 990feaf
Copy full SHA for 990feaf

File tree

Expand file treeCollapse file tree

3 files changed

+26
-9
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

3 files changed

+26
-9
lines changed
Open diff view settings
Collapse file

‎src/node_crypto.cc‎

Copy file name to clipboardExpand all lines: src/node_crypto.cc
+10-6Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4634,16 +4634,20 @@ void Hash::HashDigest(const FunctionCallbackInfo<Value>& args) {
46344634
encoding = ParseEncoding(env->isolate(), args[0], BUFFER);
46354635
}
46364636

4637-
unsigned char md_value[EVP_MAX_MD_SIZE];
4638-
unsigned int md_len;
4639-
4640-
EVP_DigestFinal_ex(hash->mdctx_.get(), md_value, &md_len);
4637+
if (hash->md_len_ == 0) {
4638+
// Some hash algorithms such as SHA3 do not support calling
4639+
// EVP_DigestFinal_ex more than once, however, Hash._flush
4640+
// and Hash.digest can both be used to retrieve the digest,
4641+
// so we need to cache it.
4642+
// See https://github.com/nodejs/node/issues/28245.
4643+
EVP_DigestFinal_ex(hash->mdctx_.get(), hash->md_value_, &hash->md_len_);
4644+
}
46414645

46424646
Local<Value> error;
46434647
MaybeLocal<Value> rc =
46444648
StringBytes::Encode(env->isolate(),
4645-
reinterpret_cast<const char*>(md_value),
4646-
md_len,
4649+
reinterpret_cast<const char*>(hash->md_value_),
4650+
hash->md_len_,
46474651
encoding,
46484652
&error);
46494653
if (rc.IsEmpty()) {
Collapse file

‎src/node_crypto.h‎

Copy file name to clipboardExpand all lines: src/node_crypto.h
+8-1Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -595,12 +595,19 @@ class Hash : public BaseObject {
595595

596596
Hash(Environment* env, v8::Local<v8::Object> wrap)
597597
: BaseObject(env, wrap),
598-
mdctx_(nullptr) {
598+
mdctx_(nullptr),
599+
md_len_(0) {
599600
MakeWeak();
600601
}
601602

603+
~Hash() override {
604+
OPENSSL_cleanse(md_value_, md_len_);
605+
}
606+
602607
private:
603608
EVPMDPointer mdctx_;
609+
unsigned char md_value_[EVP_MAX_MD_SIZE];
610+
unsigned int md_len_;
604611
};
605612

606613
class SignBase : public BaseObject {
Collapse file

‎test/parallel/test-crypto-hash-stream-pipe.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-crypto-hash-stream-pipe.js
+8-2Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,17 @@ const crypto = require('crypto');
3030

3131
const stream = require('stream');
3232
const s = new stream.PassThrough();
33-
const h = crypto.createHash('sha1');
34-
const expect = '15987e60950cf22655b9323bc1e281f9c4aff47e';
33+
const h = crypto.createHash('sha3-512');
34+
const expect = '36a38a2a35e698974d4e5791a3f05b05' +
35+
'198235381e864f91a0e8cd6a26b677ec' +
36+
'dcde8e2b069bd7355fabd68abd6fc801' +
37+
'19659f25e92f8efc961ee3a7c815c758';
3538

3639
s.pipe(h).on('data', common.mustCall(function(c) {
3740
assert.strictEqual(c, expect);
41+
// Calling digest() after piping into a stream with SHA3 should not cause
42+
// a segmentation fault, see https://github.com/nodejs/node/issues/28245.
43+
assert.strictEqual(h.digest('hex'), expect);
3844
})).setEncoding('hex');
3945

4046
s.end('aoeu');

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.