From bff9bcddb69450a0f5bdafc70a3f89e95c1631ad Mon Sep 17 00:00:00 2001
From: Ben Noordhuis <info@bnoordhuis.nl>
Date: Fri, 7 Aug 2015 23:03:00 +0200
Subject: [PATCH] src: plug memory leaks

In a few places dynamic memory was passed to the Buffer::New() overload
that makes a copy of the input, not the one that takes ownership.

This commit is a band-aid to fix the memory leaks.  Longer term, we
should look into using C++11 move semantics more effectively.

Fixes: https://github.com/nodejs/node/issues/2308
PR-URL: https://github.com/nodejs/node/pull/2352
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
---
 src/node_buffer.h  | 5 +++++
 src/node_crypto.cc | 6 +++---
 src/stream_wrap.cc | 2 +-
 src/udp_wrap.cc    | 2 +-
 4 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/src/node_buffer.h b/src/node_buffer.h
index 5b935b8c063abc..f7c88f60e03dea 100644
--- a/src/node_buffer.h
+++ b/src/node_buffer.h
@@ -67,12 +67,17 @@ static inline bool IsWithinBounds(size_t off, size_t len, size_t max) {
 // in src/node_internals.h because of a circular dependency.
 #if defined(NODE_WANT_INTERNALS)
 v8::MaybeLocal<v8::Object> New(Environment* env, size_t size);
+// Makes a copy of |data|.
 v8::MaybeLocal<v8::Object> New(Environment* env, const char* data, size_t len);
+// Takes ownership of |data|.
 v8::MaybeLocal<v8::Object> New(Environment* env,
                                char* data,
                                size_t length,
                                FreeCallback callback,
                                void* hint);
+// Takes ownership of |data|.  Must allocate |data| with malloc() or realloc()
+// because ArrayBufferAllocator::Free() deallocates it again with free().
+// Mixing operator new and free() is undefined behavior so don't do that.
 v8::MaybeLocal<v8::Object> Use(Environment* env, char* data, size_t length);
 #endif  // defined(NODE_WANT_INTERNALS)
 
diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index e33b249242e3e5..4e649e10688d41 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -4477,7 +4477,7 @@ void ECDH::ComputeSecret(const FunctionCallbackInfo<Value>& args) {
     return env->ThrowError("Failed to compute ECDH key");
   }
 
-  Local<Object> buf = Buffer::New(env, out, out_len).ToLocalChecked();
+  Local<Object> buf = Buffer::Use(env, out, out_len).ToLocalChecked();
   args.GetReturnValue().Set(buf);
 }
 
@@ -4542,7 +4542,7 @@ void ECDH::GetPrivateKey(const FunctionCallbackInfo<Value>& args) {
   }
 
   Local<Object> buf =
-      Buffer::New(env, reinterpret_cast<char*>(out), size).ToLocalChecked();
+      Buffer::Use(env, reinterpret_cast<char*>(out), size).ToLocalChecked();
   args.GetReturnValue().Set(buf);
 }
 
@@ -4945,7 +4945,7 @@ void RandomBytesCheck(RandomBytesRequest* req, Local<Value> argv[2]) {
     size_t size;
     req->return_memory(&data, &size);
     argv[0] = Null(req->env()->isolate());
-    argv[1] = Buffer::New(req->env(), data, size).ToLocalChecked();
+    argv[1] = Buffer::Use(req->env(), data, size).ToLocalChecked();
   }
 }
 
diff --git a/src/stream_wrap.cc b/src/stream_wrap.cc
index ae941f5edbd90b..3097eac4859761 100644
--- a/src/stream_wrap.cc
+++ b/src/stream_wrap.cc
@@ -223,7 +223,7 @@ void StreamWrap::OnReadImpl(ssize_t nread,
     CHECK_EQ(pending, UV_UNKNOWN_HANDLE);
   }
 
-  Local<Object> obj = Buffer::New(env, base, nread).ToLocalChecked();
+  Local<Object> obj = Buffer::Use(env, base, nread).ToLocalChecked();
   wrap->EmitData(nread, obj, pending_obj);
 }
 
diff --git a/src/udp_wrap.cc b/src/udp_wrap.cc
index dd3958ec0e37da..791ef76848cf43 100644
--- a/src/udp_wrap.cc
+++ b/src/udp_wrap.cc
@@ -408,7 +408,7 @@ void UDPWrap::OnRecv(uv_udp_t* handle,
   }
 
   char* base = static_cast<char*>(realloc(buf->base, nread));
-  argv[2] = Buffer::New(env, base, nread).ToLocalChecked();
+  argv[2] = Buffer::Use(env, base, nread).ToLocalChecked();
   argv[3] = AddressToJS(env, addr);
   wrap->MakeCallback(env->onmessage_string(), ARRAY_SIZE(argv), argv);
 }