1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

src: fix memory leak in DH key setters

Browse Source 
Fix a memory leak in dh.setPublicKey() and dh.setPrivateKey() where the
old keys weren't freed.

Fixes: https://github.com/nodejs/node/issues/8377
PR-URL: https://github.com/nodejs/node/pull/14122
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit is contained in:
Ben Noordhuis 2017-07-10 12:56:37 +02:00
parent 58926d4075
commit aa1c8c0043
3 changed files with 54 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

56
src/node_crypto.cc
View File

@ -4881,44 +4881,40 @@ void DiffieHellman::ComputeSecret(const FunctionCallbackInfo<Value>& args) {
} }
void DiffieHellman::SetPublicKey(const FunctionCallbackInfo<Value>& args) { void DiffieHellman::SetKey(const v8::FunctionCallbackInfo<v8::Value>& args,
DiffieHellman* diffieHellman; BIGNUM* (DH::*field), const char* what) {
ASSIGN_OR_RETURN_UNWRAP(&diffieHellman, args.Holder()); Environment* env = Environment::GetCurrent(args);
Environment* env = diffieHellman->env();
if (!diffieHellman->initialised_) { DiffieHellman* dh;
return ThrowCryptoError(env, ERR_get_error(), "Not initialized"); ASSIGN_OR_RETURN_UNWRAP(&dh, args.Holder());
} if (!dh->initialised_) return env->ThrowError("Not initialized");
BIGNUM** num = &((dh->dh)->*field);
char errmsg[64];
if (args.Length() == 0) { if (args.Length() == 0) {
return env->ThrowError("Public key argument is mandatory"); snprintf(errmsg, sizeof(errmsg), "%s argument is mandatory", what);
} else { return env->ThrowError(errmsg);
THROW_AND_RETURN_IF_NOT_BUFFER(args[0], "Public key");
diffieHellman->dh->pub_key = BN_bin2bn(
reinterpret_cast<unsigned char*>(Buffer::Data(args[0])),
Buffer::Length(args[0]), 0);
} }
if (!Buffer::HasInstance(args[0])) {
snprintf(errmsg, sizeof(errmsg), "%s must be a buffer", what);
return env->ThrowTypeError(errmsg);
}
*num = BN_bin2bn(reinterpret_cast<unsigned char*>(Buffer::Data(args[0])),
Buffer::Length(args[0]), *num);
CHECK_NE(*num, nullptr);
}
void DiffieHellman::SetPublicKey(const FunctionCallbackInfo<Value>& args) {
SetKey(args, &DH::pub_key, "Public key");
} }
void DiffieHellman::SetPrivateKey(const FunctionCallbackInfo<Value>& args) { void DiffieHellman::SetPrivateKey(const FunctionCallbackInfo<Value>& args) {
DiffieHellman* diffieHellman; SetKey(args, &DH::priv_key, "Private key");
ASSIGN_OR_RETURN_UNWRAP(&diffieHellman, args.Holder());
Environment* env = diffieHellman->env();
if (!diffieHellman->initialised_) {
return ThrowCryptoError(env, ERR_get_error(), "Not initialized");
}
if (args.Length() == 0) {
return env->ThrowError("Private key argument is mandatory");
} else {
THROW_AND_RETURN_IF_NOT_BUFFER(args[0], "Private key");
diffieHellman->dh->priv_key = BN_bin2bn(
reinterpret_cast<unsigned char*>(Buffer::Data(args[0])),
Buffer::Length(args[0]),
0);
}
} }

2
src/node_crypto.h
View File

@ -692,6 +692,8 @@ class DiffieHellman : public BaseObject {
private: private:
static void GetField(const v8::FunctionCallbackInfo<v8::Value>& args, static void GetField(const v8::FunctionCallbackInfo<v8::Value>& args,
BIGNUM* (DH::*field), const char* err_if_null); BIGNUM* (DH::*field), const char* err_if_null);
static void SetKey(const v8::FunctionCallbackInfo<v8::Value>& args,
BIGNUM* (DH::*field), const char* what);
bool VerifyContext(); bool VerifyContext();
bool initialised_; bool initialised_;

26
test/parallel/test-crypto-dh-leak.js Normal file
View File

@ -0,0 +1,26 @@
// Flags: --expose-gc
'use strict';
const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');
const assert = require('assert');
const crypto = require('crypto');
const before = process.memoryUsage().rss;
{
const dh = crypto.createDiffieHellman(common.hasFipsCrypto ? 1024 : 256);
const publicKey = dh.generateKeys();
const privateKey = dh.getPrivateKey();
for (let i = 0; i < 5e4; i += 1) {
dh.setPublicKey(publicKey);
dh.setPrivateKey(privateKey);
}
}
global.gc();
const after = process.memoryUsage().rss;
// RSS should stay the same, ceteris paribus, but allow for
// some slop because V8 mallocs memory during execution.
assert(after - before < 5 << 20);