Skip to content

Commit

Permalink
fix: allocate memory of Buffer with V8's allocator
Browse files Browse the repository at this point in the history 
Blink overrides ArrayBuffer's allocator with its own one, while Node
simply uses malloc and free. This commit prevents the crash that would
be resultant of mixing them together.
  • Loading branch information
@codebytere
codebytere committed Oct 2, 2018
1 parent 10df34b commit e66aa77
Show file tree
Hide file tree
Showing 7 changed files with 134 additions and 61 deletions.
28 changes: 17 additions & 11 deletions src/node_buffer.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,10 @@ bool zero_fill_all_buffers = false;

namespace {

inline void* BufferMalloc(size_t length) {
return zero_fill_all_buffers ? node::UncheckedCalloc(length) :
node::UncheckedMalloc(length);
inline void* BufferMalloc(v8::Isolate* isolate, size_t length) {
auto* allocator = isolate->GetArrayBufferAllocator();
return zero_fill_all_buffers ? allocator->Allocate(length) :
allocator->AllocateUninitialized(length);
}

} // namespace
Expand Down Expand Up @@ -245,7 +246,7 @@ MaybeLocal<Object> New(Isolate* isolate,
char* data = nullptr;

if (length > 0) {
data = static_cast<char*>(BufferMalloc(length));
data = static_cast<char*>(BufferMalloc(isolate, length));

if (data == nullptr)
return Local<Object>();
Expand All @@ -254,10 +255,14 @@ MaybeLocal<Object> New(Isolate* isolate,
CHECK(actual <= length);

if (actual == 0) {
free(data);
isolate->GetArrayBufferAllocator()->Free(data, length);
data = nullptr;
} else if (actual < length) {
data = node::Realloc(data, actual);
auto* allocator = isolate->GetArrayBufferAllocator();
auto* excessive_data = data;
data = static_cast<char*>(allocator->AllocateUninitialized(actual));
memcpy(data, excessive_data, actual);
allocator->Free(excessive_data, length);
}
}

Expand All @@ -266,7 +271,7 @@ MaybeLocal<Object> New(Isolate* isolate,
return scope.Escape(buf);

// Object failed to be created. Clean up resources.
free(data);
isolate->GetArrayBufferAllocator()->Free(data, length);
return Local<Object>();
}

Expand All @@ -292,7 +297,7 @@ MaybeLocal<Object> New(Environment* env, size_t length) {

void* data;
if (length > 0) {
data = BufferMalloc(length);
data = BufferMalloc(env->isolate(), length);
if (data == nullptr)
return Local<Object>();
} else {
Expand All @@ -308,7 +313,7 @@ MaybeLocal<Object> New(Environment* env, size_t length) {

if (ui.IsEmpty()) {
// Object failed to be created. Clean up resources.
free(data);
env->isolate()->GetArrayBufferAllocator()->Free(data, length);
}

return scope.Escape(ui.FromMaybe(Local<Uint8Array>()));
Expand All @@ -334,10 +339,11 @@ MaybeLocal<Object> Copy(Environment* env, const char* data, size_t length) {
return Local<Object>();
}

auto* allocator = env->isolate()->GetArrayBufferAllocator();
void* new_data;
if (length > 0) {
CHECK_NOT_NULL(data);
new_data = node::UncheckedMalloc(length);
new_data = allocator->AllocateUninitialized(length);
if (new_data == nullptr)
return Local<Object>();
memcpy(new_data, data, length);
Expand All @@ -354,7 +360,7 @@ MaybeLocal<Object> Copy(Environment* env, const char* data, size_t length) {

if (ui.IsEmpty()) {
// Object failed to be created. Clean up resources.
free(new_data);
allocator->Free(new_data, length);
}

return scope.Escape(ui.FromMaybe(Local<Uint8Array>()));
Expand Down
77 changes: 50 additions & 27 deletions src/node_crypto.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1865,7 +1865,8 @@ void SSLWrap<Base>::GetFinished(const FunctionCallbackInfo<Value>& args) {
if (len == 0)
return;

char* buf = Malloc(len);
auto* allocator = env->isolate()->GetArrayBufferAllocator();
char* buf = static_cast<char*>(allocator->AllocateUninitialized(len));
CHECK_EQ(len, SSL_get_finished(w->ssl_.get(), buf, len));
args.GetReturnValue().Set(Buffer::New(env, buf, len).ToLocalChecked());
}
Expand All @@ -1888,7 +1889,8 @@ void SSLWrap<Base>::GetPeerFinished(const FunctionCallbackInfo<Value>& args) {
if (len == 0)
return;

char* buf = Malloc(len);
auto* allocator = env->isolate()->GetArrayBufferAllocator();
char* buf = static_cast<char*>(allocator->AllocateUninitialized(len));
CHECK_EQ(len, SSL_get_peer_finished(w->ssl_.get(), buf, len));
args.GetReturnValue().Set(Buffer::New(env, buf, len).ToLocalChecked());
}
Expand All @@ -1908,7 +1910,8 @@ void SSLWrap<Base>::GetSession(const FunctionCallbackInfo<Value>& args) {
int slen = i2d_SSL_SESSION(sess, nullptr);
CHECK_GT(slen, 0);

char* sbuf = Malloc(slen);
auto* allocator = env->isolate()->GetArrayBufferAllocator();
char* sbuf = static_cast<char*>(allocator->AllocateUninitialized(slen));
unsigned char* p = reinterpret_cast<unsigned char*>(sbuf);
i2d_SSL_SESSION(sess, &p);
args.GetReturnValue().Set(Buffer::New(env, sbuf, slen).ToLocalChecked());
Expand Down Expand Up @@ -2329,11 +2332,12 @@ int SSLWrap<Base>::TLSExtStatusCallback(SSL* s, void* arg) {
size_t len = Buffer::Length(obj);

// OpenSSL takes control of the pointer after accepting it
char* data = node::Malloc(len);
auto* allocator = env->isolate()->GetArrayBufferAllocator();
uint8_t* data = static_cast<uint8_t*>(allocator->AllocateUninitialized(len));
memcpy(data, resp, len);

if (!SSL_set_tlsext_status_ocsp_resp(s, data, len))
free(data);
allocator->Free(data, len);
w->ocsp_response_.Reset();

return SSL_TLSEXT_ERR_OK;
Expand Down Expand Up @@ -3037,7 +3041,8 @@ CipherBase::UpdateResult CipherBase::Update(const char* data,
return kErrorState;
}

*out = Malloc<unsigned char>(buff_len);
auto* allocator = env()->isolate()->GetArrayBufferAllocator();
*out = static_cast<unsigned char*>(allocator->AllocateUninitialized(buff_len));
int r = EVP_CipherUpdate(ctx_.get(),
*out,
out_len,
Expand Down Expand Up @@ -3080,7 +3085,8 @@ void CipherBase::Update(const FunctionCallbackInfo<Value>& args) {
}

if (r != kSuccess) {
free(out);
auto* allocator = env->isolate()->GetArrayBufferAllocator();
allocator->Free(out, out_len);
if (r == kErrorState) {
ThrowCryptoError(env, ERR_get_error(),
"Trying to add data in unsupported state");
Expand Down Expand Up @@ -3119,8 +3125,9 @@ bool CipherBase::Final(unsigned char** out, int* out_len) {

const int mode = EVP_CIPHER_CTX_mode(ctx_.get());

*out = Malloc<unsigned char>(
static_cast<size_t>(EVP_CIPHER_CTX_block_size(ctx_.get())));
auto* allocator = env()->isolate()->GetArrayBufferAllocator();
*out = static_cast<unsigned char*>(allocator->AllocateUninitialized(
EVP_CIPHER_CTX_block_size(ctx_.get())));

if (kind_ == kDecipher && IsSupportedAuthenticatedMode(mode)) {
MaybePassAuthTagToOpenSSL();
Expand Down Expand Up @@ -3169,7 +3176,8 @@ void CipherBase::Final(const FunctionCallbackInfo<Value>& args) {
bool r = cipher->Final(&out_value, &out_len);

if (out_len <= 0 || !r) {
free(out_value);
auto* allocator = env->isolate()->GetArrayBufferAllocator();
allocator->Free(out_value, out_len);
out_value = nullptr;
out_len = 0;
if (!r) {
Expand Down Expand Up @@ -3818,7 +3826,8 @@ void Verify::VerifyFinal(const FunctionCallbackInfo<Value>& args) {
template <PublicKeyCipher::Operation operation,
PublicKeyCipher::EVP_PKEY_cipher_init_t EVP_PKEY_cipher_init,
PublicKeyCipher::EVP_PKEY_cipher_t EVP_PKEY_cipher>
bool PublicKeyCipher::Cipher(const char* key_pem,
bool PublicKeyCipher::Cipher(Environment* env,
const char* key_pem,
int key_pem_len,
const char* passphrase,
int padding,
Expand All @@ -3827,6 +3836,7 @@ bool PublicKeyCipher::Cipher(const char* key_pem,
unsigned char** out,
size_t* out_len) {
EVPKeyPointer pkey;
auto* allocator = env->isolate()->GetArrayBufferAllocator();

// Check if this is a PKCS#8 or RSA public key before trying as X.509 and
// private key.
Expand Down Expand Up @@ -3859,7 +3869,7 @@ bool PublicKeyCipher::Cipher(const char* key_pem,
if (EVP_PKEY_cipher(ctx.get(), nullptr, out_len, data, len) <= 0)
return false;

*out = Malloc<unsigned char>(*out_len);
*out = static_cast<unsigned char*>(allocator->AllocateUninitialized(*out_len));

if (EVP_PKEY_cipher(ctx.get(), *out, out_len, data, len) <= 0)
return false;
Expand Down Expand Up @@ -3893,6 +3903,7 @@ void PublicKeyCipher::Cipher(const FunctionCallbackInfo<Value>& args) {
ClearErrorOnReturn clear_error_on_return;

bool r = Cipher<operation, EVP_PKEY_cipher_init, EVP_PKEY_cipher>(
env,
kbuf,
klen,
args.Length() >= 4 && !args[3]->IsNull() ? *passphrase : nullptr,
Expand All @@ -3903,7 +3914,8 @@ void PublicKeyCipher::Cipher(const FunctionCallbackInfo<Value>& args) {
&out_len);

if (out_len == 0 || !r) {
free(out_value);
auto* allocator = env->isolate()->GetArrayBufferAllocator();
allocator->Free(out_value, out_len);
out_value = nullptr;
out_len = 0;
if (!r) {
Expand Down Expand Up @@ -4116,7 +4128,8 @@ void DiffieHellman::GenerateKeys(const FunctionCallbackInfo<Value>& args) {
const BIGNUM* pub_key;
DH_get0_key(diffieHellman->dh_.get(), &pub_key, nullptr);
size_t size = BN_num_bytes(pub_key);
char* data = Malloc(size);
auto* allocator = env->isolate()->GetArrayBufferAllocator();
char* data = static_cast<char*>(allocator->AllocateUninitialized(size));
BN_bn2bin(pub_key, reinterpret_cast<unsigned char*>(data));
args.GetReturnValue().Set(Buffer::New(env, data, size).ToLocalChecked());
}
Expand All @@ -4135,7 +4148,8 @@ void DiffieHellman::GetField(const FunctionCallbackInfo<Value>& args,
if (num == nullptr) return env->ThrowError(err_if_null);

size_t size = BN_num_bytes(num);
char* data = Malloc(size);
auto* allocator = env->isolate()->GetArrayBufferAllocator();
char* data = static_cast<char*>(allocator->AllocateUninitialized(size));
BN_bn2bin(num, reinterpret_cast<unsigned char*>(data));
args.GetReturnValue().Set(Buffer::New(env, data, size).ToLocalChecked());
}
Expand Down Expand Up @@ -4199,7 +4213,8 @@ void DiffieHellman::ComputeSecret(const FunctionCallbackInfo<Value>& args) {
Buffer::Length(args[0]),
0));

MallocedBuffer<char> data(DH_size(diffieHellman->dh_.get()));
auto* allocator = env->isolate()->GetArrayBufferAllocator();
MallocedBuffer<char> data(DH_size(diffieHellman->dh_.get()), allocator);

int size = DH_compute_key(reinterpret_cast<unsigned char*>(data.data),
key.get(),
Expand Down Expand Up @@ -4419,13 +4434,14 @@ void ECDH::ComputeSecret(const FunctionCallbackInfo<Value>& args) {
}

// NOTE: field_size is in bits
auto* allocator = env->isolate()->GetArrayBufferAllocator();
int field_size = EC_GROUP_get_degree(ecdh->group_);
size_t out_len = (field_size + 7) / 8;
char* out = node::Malloc(out_len);
char* out = static_cast<char*>(allocator->AllocateUninitialized(out_len));

int r = ECDH_compute_key(out, out_len, pub.get(), ecdh->key_.get(), nullptr);
if (!r) {
free(out);
allocator->Free(out, out_len);
return env->ThrowError("Failed to compute ECDH key");
}

Expand Down Expand Up @@ -4456,11 +4472,13 @@ void ECDH::GetPublicKey(const FunctionCallbackInfo<Value>& args) {
if (size == 0)
return env->ThrowError("Failed to get public key length");

unsigned char* out = node::Malloc<unsigned char>(size);
auto* allocator = env->isolate()->GetArrayBufferAllocator();
unsigned char* out =
static_cast<unsigned char*>(allocator->AllocateUninitialized(size));

int r = EC_POINT_point2oct(ecdh->group_, pub, form, out, size, nullptr);
if (r != size) {
free(out);
allocator->Free(out, size);
return env->ThrowError("Failed to get public key");
}

Expand All @@ -4480,11 +4498,13 @@ void ECDH::GetPrivateKey(const FunctionCallbackInfo<Value>& args) {
if (b == nullptr)
return env->ThrowError("Failed to get ECDH private key");

auto* allocator = env->isolate()->GetArrayBufferAllocator();
int size = BN_num_bytes(b);
unsigned char* out = node::Malloc<unsigned char>(size);
unsigned char* out =
static_cast<unsigned char*>(allocator->AllocateUninitialized(size));

if (size != BN_bn2bin(b, out)) {
free(out);
allocator->Free(out, size);
return env->ThrowError("Failed to convert ECDH private key to Buffer");
}

Expand Down Expand Up @@ -4953,8 +4973,9 @@ void VerifySpkac(const FunctionCallbackInfo<Value>& args) {
}


char* ExportPublicKey(const char* data, int len, size_t* size) {
char* ExportPublicKey(Environment* env, const char* data, int len, size_t* size) {
char* buf = nullptr;
auto* allocator = env->isolate()->GetArrayBufferAllocator();

BIOPointer bio(BIO_new(BIO_s_mem()));
if (!bio)
Expand All @@ -4975,7 +4996,7 @@ char* ExportPublicKey(const char* data, int len, size_t* size) {
BIO_get_mem_ptr(bio.get(), &ptr);

*size = ptr->length;
buf = Malloc(*size);
buf = static_cast<char*>(allocator->AllocateUninitialized(*size));
memcpy(buf, ptr->data, *size);

return buf;
Expand All @@ -4993,7 +5014,7 @@ void ExportPublicKey(const FunctionCallbackInfo<Value>& args) {
CHECK_NOT_NULL(data);

size_t pkey_size;
char* pkey = ExportPublicKey(data, length, &pkey_size);
char* pkey = ExportPublicKey(env, data, length, &pkey_size);
if (pkey == nullptr)
return args.GetReturnValue().SetEmptyString();

Expand Down Expand Up @@ -5075,11 +5096,13 @@ void ConvertKey(const FunctionCallbackInfo<Value>& args) {
if (size == 0)
return env->ThrowError("Failed to get public key length");

unsigned char* out = node::Malloc<unsigned char>(size);
auto* allocator = env->isolate()->GetArrayBufferAllocator();
unsigned char* out =
static_cast<unsigned char*>(allocator->AllocateUninitialized(size));

int r = EC_POINT_point2oct(group.get(), pub.get(), form, out, size, nullptr);
if (r != size) {
free(out);
allocator->Free(out, size);
return env->ThrowError("Failed to get public key");
}

Expand Down
3 changes: 2 additions & 1 deletion src/node_crypto.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,8 @@ class PublicKeyCipher {
template <Operation operation,
EVP_PKEY_cipher_init_t EVP_PKEY_cipher_init,
EVP_PKEY_cipher_t EVP_PKEY_cipher>
static bool Cipher(const char* key_pem,
static bool Cipher(Environment* env,
const char* key_pem,
int key_pem_len,
const char* passphrase,
int padding,
Expand Down
Loading

0 comments on commit e66aa77

Please sign in to comment.