From 7c5d3296e793562698a34182ae8556e34330e589 Mon Sep 17 00:00:00 2001 From: Vadim Macagon Date: Sun, 2 Oct 2016 23:38:39 +0700 Subject: [PATCH] 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. --- atom/browser/atom_blob_reader.cc | 7 ++++++- .../printing/print_preview_message_handler.cc | 12 ++++++++---- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/atom/browser/atom_blob_reader.cc b/atom/browser/atom_blob_reader.cc index 360024a0743f..676386bc6af7 100644 --- 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 buffer = node::Buffer::New(isolate, - blob_data, static_cast(size)).ToLocalChecked(); + blob_data, static_cast(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 d1f3660f8d2d..91a6c5167c4c 100644 --- 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(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 buffer = node::Buffer::New(isolate, - data, static_cast(data_size)).ToLocalChecked(); + data, static_cast(data_size), &FreeNodeBufferData, nullptr) + .ToLocalChecked(); print_to_pdf_callback_map_[request_id].Run(v8::Null(isolate), buffer); } else { v8::Local 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)); }