Skip to content
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.

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

src: return static buffer on malloc(0) #8658

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions src/cares_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<uv_poll_t*>(watcher));
free(task);
node::Free(task);
}


Expand All @@ -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;
}

Expand Down
10 changes: 5 additions & 5 deletions src/node_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ MaybeLocal<Object> New(Isolate* isolate,
CHECK(actual <= length);

if (actual == 0) {
free(data);
node::Free(data);
data = nullptr;
} else if (actual < length) {
data = static_cast<char*>(node::Realloc(data, actual));
Expand All @@ -288,7 +288,7 @@ MaybeLocal<Object> New(Isolate* isolate,
return scope.Escape(buf);

// Object failed to be created. Clean up resources.
free(data);
node::Free(data);
return Local<Object>();
}

Expand Down Expand Up @@ -331,7 +331,7 @@ MaybeLocal<Object> 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<Object>();
}

Expand Down Expand Up @@ -377,7 +377,7 @@ MaybeLocal<Object> 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<Object>();
}

Expand Down Expand Up @@ -1092,7 +1092,7 @@ void IndexOfString(const FunctionCallbackInfo<Value>& args) {
needle_length,
offset,
is_forward);
free(needle_data);
node::Free(needle_data);
}

args.GetReturnValue().Set(
Expand Down
24 changes: 12 additions & 12 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2284,7 +2284,7 @@ int SSLWrap<Base>::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;
Expand Down Expand Up @@ -4912,7 +4912,7 @@ void ECDH::ComputeSecret(const FunctionCallbackInfo<Value>& 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");
}

Expand Down Expand Up @@ -4947,7 +4947,7 @@ void ECDH::GetPublicKey(const FunctionCallbackInfo<Value>& 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");
}

Expand All @@ -4972,7 +4972,7 @@ void ECDH::GetPrivateKey(const FunctionCallbackInfo<Value>& 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");
}

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -5354,8 +5354,8 @@ void PBKDF2(const FunctionCallbackInfo<Value>& args) {
return;

err:
free(salt);
free(pass);
node::Free(salt);
node::Free(pass);
return env->ThrowTypeError(type_error);
}

Expand All @@ -5368,7 +5368,7 @@ class RandomBytesRequest : public AsyncWrap {
error_(0),
size_(size),
data_(static_cast<char*>(node::Malloc(size))) {
if (data() == nullptr && size > 0)
if (data() == nullptr)
FatalError("node::RandomBytesRequest()", "Out of Memory");
Wrap(object, this);
}
Expand All @@ -5391,7 +5391,7 @@ class RandomBytesRequest : public AsyncWrap {
}

inline void release() {
free(data_);
node::Free(data_);
size_ = 0;
}

Expand Down Expand Up @@ -5606,7 +5606,7 @@ void GetCurves(const FunctionCallbackInfo<Value>& args) {
}
}

free(curves);
node::Free(curves);
}

args.GetReturnValue().Set(arr);
Expand Down
2 changes: 1 addition & 1 deletion src/node_internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
8 changes: 3 additions & 5 deletions src/stream_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ void StreamWrap::OnAllocImpl(size_t size, uv_buf_t* buf, void* ctx) {
buf->base = static_cast<char*>(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");
Expand Down Expand Up @@ -192,15 +192,13 @@ void StreamWrap::OnReadImpl(ssize_t nread,
Local<Object> pending_obj;

if (nread < 0) {
if (buf->base != nullptr)
free(buf->base);
node::Free(buf->base);
wrap->EmitData(nread, Local<Object>(), pending_obj);
return;
}

if (nread == 0) {
if (buf->base != nullptr)
free(buf->base);
node::Free(buf->base);
return;
}

Expand Down
8 changes: 4 additions & 4 deletions src/string_bytes.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ template <typename ResourceType, typename TypeName>
class ExternString: public ResourceType {
public:
~ExternString() override {
free(const_cast<TypeName*>(data_));
node::Free(const_cast<TypeName*>(data_));
isolate()->AdjustAmountOfExternalAllocatedMemory(-byte_length());
}

Expand Down Expand Up @@ -631,7 +631,7 @@ Local<Value> 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);
}
Expand Down Expand Up @@ -669,7 +669,7 @@ Local<Value> 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);
}
Expand All @@ -687,7 +687,7 @@ Local<Value> 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);
}
Expand Down
8 changes: 3 additions & 5 deletions src/udp_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ void UDPWrap::OnAlloc(uv_handle_t* handle,
buf->base = static_cast<char*>(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");
}
Expand All @@ -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;
}

Expand All @@ -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;
}
Expand Down
20 changes: 16 additions & 4 deletions src/util-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -234,27 +234,39 @@ 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);
}

// 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
Expand Down
3 changes: 3 additions & 0 deletions src/util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 <typename T>
static void MakeUtf8String(Isolate* isolate,
Local<Value> value,
Expand Down
5 changes: 3 additions & 2 deletions src/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
8 changes: 8 additions & 0 deletions test/parallel/test-crypto-pbkdf2.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}));
});