Skip to content

crypto: fix memory leak, reduce heap allocations, clean up #14122

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
bnoordhuis wants to merge 18 commits into from
Closed

crypto: fix memory leak, reduce heap allocations, clean up #14122

Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
a9d1431
src: remove extra heap allocations in CipherBase
bnoordhuis Jul 10, 2017
df1e3e8
src: remove unneeded const_cast
bnoordhuis Jul 10, 2017
87fc516
src: avoid heap allocation in sign.final()
bnoordhuis Jul 10, 2017
647370a
src: don't heap allocate GCM cipher auth tag
bnoordhuis Jul 10, 2017
f46e980
src: remove superfluous cipher_ data member
bnoordhuis Jul 10, 2017
5ae2cb8
src: remove unused Local
bnoordhuis Jul 10, 2017
dd6f4a9
src: make CipherBase::kind_ const
bnoordhuis Jul 10, 2017
d383042
src: remove extra heap allocation in GetSession()
bnoordhuis Jul 10, 2017
496ef4b
src: avoid heap allocation in hmac.digest()
bnoordhuis Jul 10, 2017
40807c1
src: remove extra heap allocations in DH functions
bnoordhuis Jul 10, 2017
5fd09df
src: refactor PBKDF2Request
bnoordhuis Jul 10, 2017
160b60f
src: make array arg length compile-time checkable
bnoordhuis Jul 10, 2017
3818792
src: avoid heap allocation in crypto.pbkdf2()
bnoordhuis Jul 10, 2017
0ce90a9
src: remove PBKDF2Request::release()
bnoordhuis Jul 10, 2017
1cfd02e
src: simplify PBKDF2Request
bnoordhuis Jul 10, 2017
b8e3a3e
src: guard against double free in randomBytes()
bnoordhuis Jul 10, 2017
083ccd2
src: reduce allocations in exportPublicKey()
bnoordhuis Jul 10, 2017
e4ac1b6
src: fix memory leak in DH key setters
bnoordhuis Jul 10, 2017
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
src: remove extra heap allocations in CipherBase 
Don't allocate + copy + free; allocate and fill in place, then hand off
the pointer to Buffer::New().

Avoids unnecessary heap allocations in the following methods:

- crypto.Cipher#final()
- crypto.Cipher#update()
- crypto.Cipheriv#final()
- crypto.Cipheriv#update()
- crypto.Decipher#final()
- crypto.Decipher#update()
- crypto.Decipheriv#final()
- crypto.Decipheriv#update()
- crypto.privateDecrypt()
- crypto.privateEncrypt()
- crypto.publicDecrypt()
- crypto.publicEncrypt()
  • Loading branch information
@bnoordhuis
bnoordhuis committed Jul 17, 2017
commit a9d1431943660995656c1c04449f5bac3494faa9
28 changes: 12 additions & 16 deletions src/node_crypto.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3561,7 +3561,7 @@ bool CipherBase::Update(const char* data,
}

*out_len = len + EVP_CIPHER_CTX_block_size(&ctx_);
*out = new unsigned char[*out_len];
*out = Malloc<unsigned char>(static_cast<size_t>(*out_len));
return EVP_CipherUpdate(&ctx_,
*out,
out_len,
Expand Down Expand Up @@ -3595,17 +3595,15 @@ void CipherBase::Update(const FunctionCallbackInfo<Value>& args) {
}

if (!r) {
delete[] out;
free(out);
return ThrowCryptoError(env,
ERR_get_error(),
"Trying to add data in unsupported state");
}

CHECK(out != nullptr || out_len == 0);
Local<Object> buf =
Buffer::Copy(env, reinterpret_cast<char*>(out), out_len).ToLocalChecked();
if (out)
delete[] out;
Buffer::New(env, reinterpret_cast<char*>(out), out_len).ToLocalChecked();

args.GetReturnValue().Set(buf);
}
Expand Down Expand Up @@ -3633,7 +3631,8 @@ bool CipherBase::Final(unsigned char** out, int *out_len) {
if (!initialised_)
return false;

*out = new unsigned char[EVP_CIPHER_CTX_block_size(&ctx_)];
*out = Malloc<unsigned char>(
static_cast<size_t>(EVP_CIPHER_CTX_block_size(&ctx_)));
int r = EVP_CipherFinal_ex(&ctx_, *out, out_len);

if (r && kind_ == kCipher) {
Expand Down Expand Up @@ -3670,7 +3669,7 @@ void CipherBase::Final(const FunctionCallbackInfo<Value>& args) {
bool r = cipher->Final(&out_value, &out_len);

if (out_len <= 0 || !r) {
delete[] out_value;
free(out_value);
out_value = nullptr;
out_len = 0;
if (!r) {
Expand All @@ -3684,12 +3683,11 @@ void CipherBase::Final(const FunctionCallbackInfo<Value>& args) {
}
}

Local<Object> buf = Buffer::Copy(
Local<Object> buf = Buffer::New(
env,
reinterpret_cast<char*>(out_value),
out_len).ToLocalChecked();
args.GetReturnValue().Set(buf);
delete[] out_value;
}


Expand Down Expand Up @@ -4560,7 +4558,7 @@ bool PublicKeyCipher::Cipher(const char* key_pem,
if (EVP_PKEY_cipher(ctx, nullptr, out_len, data, len) <= 0)
goto exit;

*out = new unsigned char[*out_len];
*out = Malloc<unsigned char>(*out_len);

if (EVP_PKEY_cipher(ctx, *out, out_len, data, len) <= 0)
goto exit;
Expand Down Expand Up @@ -4615,7 +4613,7 @@ void PublicKeyCipher::Cipher(const FunctionCallbackInfo<Value>& args) {
&out_len);

if (out_len == 0 || !r) {
delete[] out_value;
free(out_value);
out_value = nullptr;
out_len = 0;
if (!r) {
Expand All @@ -4624,12 +4622,10 @@ void PublicKeyCipher::Cipher(const FunctionCallbackInfo<Value>& args) {
}
}

Local<Object> vbuf = Buffer::Copy(
env,
reinterpret_cast<char*>(out_value),
out_len).ToLocalChecked();
Local<Object> vbuf =
Buffer::New(env, reinterpret_cast<char*>(out_value), out_len)
.ToLocalChecked();
args.GetReturnValue().Set(vbuf);
delete[] out_value;
}


Expand Down