Skip to content

[v10.x Backport] Backport crypto fix to v10 #37009

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
evanlucas wants to merge 2 commits into from
Closed

[v10.x Backport] Backport crypto fix to v10 #37009

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2a0571a
crypto: fix crash when calling digest after piping
tniessen Jun 16, 2019
6858287
test: add test that verifies crypto stream pipeline
evanlucas Jan 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
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>
  • Loading branch information
@tniessen @evanlucas
tniessen authored and evanlucas committed Feb 4, 2021
commit 2a0571a6b42d62d689769a1c4d891e3323dd81c2
16 changes: 10 additions & 6 deletions src/node_crypto.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3419,16 +3419,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()) {
Expand Down
9 changes: 8 additions & 1 deletion src/node_crypto.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -475,12 +475,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 {
Expand Down