Prevent undefined behavior when some Node Buffer objects are destroyed

If node::Buffer::New() is used to wrap an existing chunk of memory
without providing a custom callback to release that memory then Node
will just use `free()`. In a couple of places Node buffer objects were
constructed from chunks of memory that were allocated with `new[]`, but
a custom callback to release that memory was omitted, this resulted in
undefined behavior when those buffers were destroyed because `free()`
was used to release memory allocated with `new[]`.

To avoid undefined behavior the aforementioned buffer objects are now
constructed with a custom callback that safely releases the underlying
chunk of memory.

diff --git a/atom/browser/atom_blob_reader.cc b/atom/browser/atom_blob_reader.cc
index 360024a..676386b 100644 (file)
--- a/atom/browser/atom_blob_reader.cc
+++ b/atom/browser/atom_blob_reader.cc
@@ -21,6 +21,10 @@ namespace atom {
 
 namespace {
 
+void FreeNodeBufferData(char* data, void* hint) {
+  delete[] data;
+}
+
 void RunCallbackInUI(
     const AtomBlobReader::CompletionCallback& callback,
     char* blob_data,
@@ -32,7 +36,8 @@ void RunCallbackInUI(
   v8::HandleScope handle_scope(isolate);
   if (blob_data) {
     v8::Local<v8::Value> buffer = node::Buffer::New(isolate,
-        blob_data, static_cast<size_t>(size)).ToLocalChecked();
+        blob_data, static_cast<size_t>(size), &FreeNodeBufferData, nullptr)
+        .ToLocalChecked();
     callback.Run(buffer);
   } else {
     callback.Run(v8::Null(isolate));

diff --git a/chromium_src/chrome/browser/printing/print_preview_message_handler.cc b/chromium_src/chrome/browser/printing/print_preview_message_handler.cc
index d1f3660..91a6c51 100644 (file)
--- a/chromium_src/chrome/browser/printing/print_preview_message_handler.cc
+++ b/chromium_src/chrome/browser/printing/print_preview_message_handler.cc
@@ -47,12 +47,15 @@ char* CopyPDFDataOnIOThread(
       new base::SharedMemory(params.metafile_data_handle, true));
   if (!shared_buf->Map(params.data_size))
     return nullptr;
-  char* memory_pdf_data = static_cast<char*>(shared_buf->memory());
   char* pdf_data = new char[params.data_size];
-  memcpy(pdf_data, memory_pdf_data, params.data_size);
+  memcpy(pdf_data, shared_buf->memory(), params.data_size);
   return pdf_data;
 }
 
+void FreeNodeBufferData(char* data, void* hint) {
+  delete[] data;
+}
+
 }  // namespace
 
 namespace printing {
@@ -126,11 +129,12 @@ void PrintPreviewMessageHandler::RunPrintToPDFCallback(
   v8::HandleScope handle_scope(isolate);
   if (data) {
     v8::Local<v8::Value> buffer = node::Buffer::New(isolate,
-        data, static_cast<size_t>(data_size)).ToLocalChecked();
+        data, static_cast<size_t>(data_size), &FreeNodeBufferData, nullptr)
+        .ToLocalChecked();
     print_to_pdf_callback_map_[request_id].Run(v8::Null(isolate), buffer);
   } else {
     v8::Local<v8::String> error_message = v8::String::NewFromUtf8(isolate,
-        "Fail to generate PDF");
+        "Failed to generate PDF");
     print_to_pdf_callback_map_[request_id].Run(
         v8::Exception::Error(error_message), v8::Null(isolate));
   }