diff --git a/src/cares_wrap.cc b/src/cares_wrap.cc index 186f542cec5f85..f429725ebe4b24 100644 --- a/src/cares_wrap.cc +++ b/src/cares_wrap.cc @@ -136,7 +136,7 @@ static void ares_poll_cb(uv_poll_t* watcher, int status, int events) { static void ares_poll_close_cb(uv_handle_t* watcher) { node_ares_task* task = ContainerOf(&node_ares_task::poll_watcher, reinterpret_cast(watcher)); - free(task); + node::Free(task); } @@ -155,7 +155,7 @@ static node_ares_task* ares_task_create(Environment* env, ares_socket_t sock) { if (uv_poll_init_socket(env->event_loop(), &task->poll_watcher, sock) < 0) { /* This should never happen. */ - free(task); + node::Free(task); return nullptr; } diff --git a/src/node_buffer.cc b/src/node_buffer.cc index 4baa8d9ac659b8..02e764018c9aed 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -275,7 +275,7 @@ MaybeLocal New(Isolate* isolate, CHECK(actual <= length); if (actual == 0) { - free(data); + node::Free(data); data = nullptr; } else if (actual < length) { data = static_cast(node::Realloc(data, actual)); @@ -288,7 +288,7 @@ MaybeLocal New(Isolate* isolate, return scope.Escape(buf); // Object failed to be created. Clean up resources. - free(data); + node::Free(data); return Local(); } @@ -331,7 +331,7 @@ MaybeLocal New(Environment* env, size_t length) { return scope.Escape(ui); // Object failed to be created. Clean up resources. - free(data); + node::Free(data); return Local(); } @@ -377,7 +377,7 @@ MaybeLocal Copy(Environment* env, const char* data, size_t length) { return scope.Escape(ui); // Object failed to be created. Clean up resources. - free(new_data); + node::Free(new_data); return Local(); } @@ -1092,7 +1092,7 @@ void IndexOfString(const FunctionCallbackInfo& args) { needle_length, offset, is_forward); - free(needle_data); + node::Free(needle_data); } args.GetReturnValue().Set( diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 33b844f07d8857..f0aad5c5381c0e 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -2284,7 +2284,7 @@ int SSLWrap::TLSExtStatusCallback(SSL* s, void* arg) { memcpy(data, resp, len); if (!SSL_set_tlsext_status_ocsp_resp(s, data, len)) - free(data); + node::Free(data); w->ocsp_response_.Reset(); return SSL_TLSEXT_ERR_OK; @@ -4912,7 +4912,7 @@ void ECDH::ComputeSecret(const FunctionCallbackInfo& args) { int r = ECDH_compute_key(out, out_len, pub, ecdh->key_, nullptr); EC_POINT_free(pub); if (!r) { - free(out); + node::Free(out); return env->ThrowError("Failed to compute ECDH key"); } @@ -4947,7 +4947,7 @@ void ECDH::GetPublicKey(const FunctionCallbackInfo& args) { int r = EC_POINT_point2oct(ecdh->group_, pub, form, out, size, nullptr); if (r != size) { - free(out); + node::Free(out); return env->ThrowError("Failed to get public key"); } @@ -4972,7 +4972,7 @@ void ECDH::GetPrivateKey(const FunctionCallbackInfo& args) { CHECK_NE(out, nullptr); if (size != BN_bn2bin(b, out)) { - free(out); + node::Free(out); return env->ThrowError("Failed to convert ECDH private key to Buffer"); } @@ -5149,15 +5149,15 @@ class PBKDF2Request : public AsyncWrap { } inline void release() { - free(pass_); + node::Free(pass_); pass_ = nullptr; passlen_ = 0; - free(salt_); + node::Free(salt_); salt_ = nullptr; saltlen_ = 0; - free(key_); + node::Free(key_); key_ = nullptr; keylen_ = 0; } @@ -5354,8 +5354,8 @@ void PBKDF2(const FunctionCallbackInfo& args) { return; err: - free(salt); - free(pass); + node::Free(salt); + node::Free(pass); return env->ThrowTypeError(type_error); } @@ -5368,7 +5368,7 @@ class RandomBytesRequest : public AsyncWrap { error_(0), size_(size), data_(static_cast(node::Malloc(size))) { - if (data() == nullptr && size > 0) + if (data() == nullptr) FatalError("node::RandomBytesRequest()", "Out of Memory"); Wrap(object, this); } @@ -5391,7 +5391,7 @@ class RandomBytesRequest : public AsyncWrap { } inline void release() { - free(data_); + node::Free(data_); size_ = 0; } @@ -5606,7 +5606,7 @@ void GetCurves(const FunctionCallbackInfo& args) { } } - free(curves); + node::Free(curves); } args.GetReturnValue().Set(arr); diff --git a/src/node_internals.h b/src/node_internals.h index de5d3a798e1019..c7f238d4632c2f 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -182,7 +182,7 @@ class ArrayBufferAllocator : public v8::ArrayBuffer::Allocator { virtual void* Allocate(size_t size); // Defined in src/node.cc virtual void* AllocateUninitialized(size_t size) { return node::Malloc(size); } - virtual void Free(void* data, size_t) { free(data); } + virtual void Free(void* data, size_t) { node::Free(data); } private: uint32_t zero_fill_field_ = 1; // Boolean but exposed as uint32 to JS land. diff --git a/src/stream_wrap.cc b/src/stream_wrap.cc index 2b50895c9f9c92..2a9c72388e02b6 100644 --- a/src/stream_wrap.cc +++ b/src/stream_wrap.cc @@ -151,7 +151,7 @@ void StreamWrap::OnAllocImpl(size_t size, uv_buf_t* buf, void* ctx) { buf->base = static_cast(node::Malloc(size)); buf->len = size; - if (buf->base == nullptr && size > 0) { + if (buf->base == nullptr) { FatalError( "node::StreamWrap::DoAlloc(size_t, uv_buf_t*, void*)", "Out Of Memory"); @@ -192,15 +192,13 @@ void StreamWrap::OnReadImpl(ssize_t nread, Local pending_obj; if (nread < 0) { - if (buf->base != nullptr) - free(buf->base); + node::Free(buf->base); wrap->EmitData(nread, Local(), pending_obj); return; } if (nread == 0) { - if (buf->base != nullptr) - free(buf->base); + node::Free(buf->base); return; } diff --git a/src/string_bytes.cc b/src/string_bytes.cc index fa641af7469d07..3ea9ba6ccb4c4f 100644 --- a/src/string_bytes.cc +++ b/src/string_bytes.cc @@ -29,7 +29,7 @@ template class ExternString: public ResourceType { public: ~ExternString() override { - free(const_cast(data_)); + node::Free(const_cast(data_)); isolate()->AdjustAmountOfExternalAllocatedMemory(-byte_length()); } @@ -631,7 +631,7 @@ Local StringBytes::Encode(Isolate* isolate, force_ascii(buf, out, buflen); if (buflen < EXTERN_APEX) { val = OneByteString(isolate, out, buflen); - free(out); + node::Free(out); } else { val = ExternOneByteString::New(isolate, out, buflen); } @@ -669,7 +669,7 @@ Local StringBytes::Encode(Isolate* isolate, if (dlen < EXTERN_APEX) { val = OneByteString(isolate, dst, dlen); - free(dst); + node::Free(dst); } else { val = ExternOneByteString::New(isolate, dst, dlen); } @@ -687,7 +687,7 @@ Local StringBytes::Encode(Isolate* isolate, if (dlen < EXTERN_APEX) { val = OneByteString(isolate, dst, dlen); - free(dst); + node::Free(dst); } else { val = ExternOneByteString::New(isolate, dst, dlen); } diff --git a/src/udp_wrap.cc b/src/udp_wrap.cc index f191b3a75540c9..179a307f61a45a 100644 --- a/src/udp_wrap.cc +++ b/src/udp_wrap.cc @@ -387,7 +387,7 @@ void UDPWrap::OnAlloc(uv_handle_t* handle, buf->base = static_cast(node::Malloc(suggested_size)); buf->len = suggested_size; - if (buf->base == nullptr && suggested_size > 0) { + if (buf->base == nullptr) { FatalError("node::UDPWrap::OnAlloc(uv_handle_t*, size_t, uv_buf_t*)", "Out Of Memory"); } @@ -400,8 +400,7 @@ void UDPWrap::OnRecv(uv_udp_t* handle, const struct sockaddr* addr, unsigned int flags) { if (nread == 0 && addr == nullptr) { - if (buf->base != nullptr) - free(buf->base); + node::Free(buf->base); return; } @@ -420,8 +419,7 @@ void UDPWrap::OnRecv(uv_udp_t* handle, }; if (nread < 0) { - if (buf->base != nullptr) - free(buf->base); + node::Free(buf->base); wrap->MakeCallback(env->onmessage_string(), arraysize(argv), argv); return; } diff --git a/src/util-inl.h b/src/util-inl.h index 5644ee90487744..40e0a2c179a6d2 100644 --- a/src/util-inl.h +++ b/src/util-inl.h @@ -234,11 +234,12 @@ bool StringEqualNoCaseN(const char* a, const char* b, size_t length) { // compiler version specific functionality. // malloc(0) and realloc(ptr, 0) have implementation-defined behavior in // that the standard allows them to either return a unique pointer or a -// nullptr for zero-sized allocation requests. Normalize by always using -// a nullptr. +// nullptr for zero-sized allocation requests. Normalize by always using +// a static buffer. +extern void* fake_mem[1]; void* Realloc(void* pointer, size_t size) { if (size == 0) { - free(pointer); + Free(pointer); return nullptr; } return realloc(pointer, size); @@ -246,15 +247,26 @@ void* Realloc(void* pointer, size_t size) { // As per spec realloc behaves like malloc if passed nullptr. void* Malloc(size_t size) { + if (size == 0) { + return fake_mem; + } return Realloc(nullptr, size); } void* Calloc(size_t n, size_t size) { - if ((n == 0) || (size == 0)) return nullptr; + if (n == 0 || size == 0) { + return fake_mem; + } CHECK_GE(n * size, n); // Overflow guard. return calloc(n, size); } +void Free(void* pointer) { + if (pointer != fake_mem) { + free(pointer); + } +} + } // namespace node #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS diff --git a/src/util.cc b/src/util.cc index 7ce99d5c76aa93..bc998275434d92 100644 --- a/src/util.cc +++ b/src/util.cc @@ -10,6 +10,9 @@ using v8::Local; using v8::String; using v8::Value; +// Used by node::Malloc, node::Calloc, node::Free +void* fake_mem[1]; + template static void MakeUtf8String(Isolate* isolate, Local value, diff --git a/src/util.h b/src/util.h index e74980d72f8e63..8f4d779110dc2a 100644 --- a/src/util.h +++ b/src/util.h @@ -29,11 +29,12 @@ namespace node { // compiler version specific functionality // malloc(0) and realloc(ptr, 0) have implementation-defined behavior in // that the standard allows them to either return a unique pointer or a -// nullptr for zero-sized allocation requests. Normalize by always using -// a nullptr. +// nullptr for zero-sized allocation requests. Normalize by always using +// a static buffer. inline void* Realloc(void* pointer, size_t size); inline void* Malloc(size_t size); inline void* Calloc(size_t n, size_t size); +inline void Free(void* pointer); #ifdef __GNUC__ #define NO_RETURN __attribute__((noreturn)) diff --git a/test/parallel/test-crypto-pbkdf2.js b/test/parallel/test-crypto-pbkdf2.js index c1897c2d69ebdd..f9fa7aa486318d 100644 --- a/test/parallel/test-crypto-pbkdf2.js +++ b/test/parallel/test-crypto-pbkdf2.js @@ -84,3 +84,11 @@ assert.throws(function() { assert.throws(function() { crypto.pbkdf2('password', 'salt', 1, 4073741824, 'sha256', common.fail); }, /Bad key length/); + +// Should not get FATAL ERROR with empty password and salt +// https://github.com/nodejs/node/issues/8571 +assert.doesNotThrow(() => { + crypto.pbkdf2('', '', 1, 32, 'sha256', common.mustCall((e) => { + assert.ifError(e); + })); +});