src: return static buffer on malloc(0)

Fixes: nodejs#8571

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<uv_poll_t*>(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;
   }
 

src/node_buffer.cc

@@ -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));
@@ -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>();
 }
 
@@ -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>();
 }
 
@@ -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>();
 }
 
@@ -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(

src/node_crypto.cc

@@ -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;
@@ -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");
   }
 
@@ -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");
   }
 
@@ -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");
   }
 
@@ -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<Value>& 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<char*>(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<Value>& args) {
       }
     }
 
-    free(curves);
+    node::Free(curves);
   }
 
   args.GetReturnValue().Set(arr);

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.

src/stream_wrap.cc

@@ -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");
@@ -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;
   }
 

src/string_bytes.cc

@@ -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());
   }
 
@@ -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);
         }
@@ -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);
       }
@@ -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);
       }

src/udp_wrap.cc

@@ -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");
   }
@@ -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;
   }

src/util-inl.h

@@ -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

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 <typename T>
 static void MakeUtf8String(Isolate* isolate,
                            Local<Value> value,

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))

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);
+  }));
+});