From 4c8209af77802ed8f2b82cdf0720349fe6b7edc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1t=C3=A9=20Szab=C3=B3?= Date: Sat, 24 May 2025 00:47:17 +0200 Subject: [PATCH] Fix mismatched allocation with empty string keys and payloads Producer::NodeProduce() creates a temporary buffer backed by a `new char[0]` for keys and payloads that were empty buffers, since node::Buffer::Data() returns NULL for these. The [NAN docs](https://github.com/nodejs/nan/blob/main/doc/buffers.md#nannewbuffer) indicate that the data managed by this buffer will be freed by free() in the absence of a custom free callback, which leads ASAN to complain: ``` ================================================================= ==28555==ERROR: AddressSanitizer: alloc-dealloc-mismatch (operator new [] vs free) on 0x60200010fc70 #0 0x7f6c0e98fb6f in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:123 #1 0x127e3d4 in v8::internal::BackingStore::~BackingStore() (/usr/bin/node+0x127e3d4) #2 0xc0934a in std::_Sp_counted_deleter, std::allocator, (__gnu_cxx::_Lock_policy)2>::_M_dispose() (/usr/bin/node+0xc0934a) #3 0x10a9fce in v8::internal::ArrayBufferSweeper::SweepingJob::SweepYoung() (/usr/bin/node+0x10a9fce) #4 0x10aab2e in std::_Function_handler::_M_invoke(std::_Any_data const&) (/usr/bin/node+0x10aab2e) #5 0xd49370 in node::(anonymous namespace)::PlatformWorkerThread(void*) (/usr/bin/node+0xd49370) #6 0x7f6c0e591ea6 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x7ea6) #7 0x7f6c0e4afa6e in __clone (/lib/x86_64-linux-gnu/libc.so.6+0xfba6e) 0x60200010fc70 is located 0 bytes inside of 1-byte region [0x60200010fc70,0x60200010fc71) allocated by thread T0 here: #0 0x7f6c0e9917a7 in operator new[](unsigned long) ../../../../src/libsanitizer/asan/asan_new_delete.cpp:102 #1 0x7f6963812e97 in NodeKafka::Producer::NodeProduce(Nan::FunctionCallbackInfo const&) ../src/producer.cc:501 #2 0x7f69637f18f7 in FunctionCallbackWrapper ../node_modules/nan/nan_callbacks_12_inl.h:177 #3 0x7f6c0430d545 (/usr/bin/node+0x14e5545) #4 0x7f6be45470df () #5 0x18e3d1b in Builtins_InterpreterEntryTrampoline (/usr/bin/node+0x18e3d1b) #6 0x7f6be453ef57 () #7 0x7f6be454d668 () #8 0x18e3d1b in Builtins_InterpreterEntryTrampoline (/usr/bin/node+0x18e3d1b) #9 0x18e3d1b in Builtins_InterpreterEntryTrampoline (/usr/bin/node+0x18e3d1b) #10 0x7f6be453ce66 () #11 0x7f6be44fdf61 () #12 0x7f6be4543bee () #13 0x18e3d1b in Builtins_InterpreterEntryTrampoline (/usr/bin/node+0x18e3d1b) #14 0x7f6be44fddea () #15 0x18e3d1b in Builtins_InterpreterEntryTrampoline (/usr/bin/node+0x18e3d1b) #16 0x18e3d1b in Builtins_InterpreterEntryTrampoline (/usr/bin/node+0x18e3d1b) #17 0x18e3d1b in Builtins_InterpreterEntryTrampoline (/usr/bin/node+0x18e3d1b) #18 0x18e3d1b in Builtins_InterpreterEntryTrampoline (/usr/bin/node+0x18e3d1b) #19 0x18e3d1b in Builtins_InterpreterEntryTrampoline (/usr/bin/node+0x18e3d1b) #20 0x18e20db in Builtins_JSEntryTrampoline (/usr/bin/node+0x18e20db) #21 0x18e1e02 in Builtins_JSEntry (/usr/bin/node+0x18e1e02) #22 0x105f17a in v8::internal::(anonymous namespace)::Invoke(v8::internal::Isolate*, v8::internal::(anonymous namespace)::InvokeParams const&) (/usr/bin/node+0x105f17a) #23 0x1060213 in v8::internal::Execution::Call(v8::internal::Isolate*, v8::internal::Handle, v8::internal::Handle, int, v8::internal::Handle*) (/usr/bin/node+0x1060213) #24 0xf22a7c in v8::Function::Call(v8::Local, v8::Local, int, v8::Local*) (/usr/bin/node+0xf22a7c) #25 0xbc911a in node::InternalMakeCallback(node::Environment*, v8::Local, v8::Local, v8::Local, int, v8::Local*, node::async_context) (/usr/bin/node+0xbc911a) #26 0xbc925d in node::MakeCallback(v8::Isolate*, v8::Local, v8::Local, int, v8::Local*, node::async_context) (/usr/bin/node+0xbc925d) #27 0x7f6963835c25 in Nan::AsyncResource::runInAsyncScope(v8::Local, v8::Local, int, v8::Local*) ../node_modules/nan/nan.h:631 #28 0x7f6963835c25 in Nan::Callback::Call_(v8::Isolate*, v8::Local, int, v8::Local*, Nan::AsyncResource*) const ../node_modules/nan/nan.h:1881 #29 0x7f6963835c25 in Nan::Callback::Call(int, v8::Local*) const ../node_modules/nan/nan.h:1825 #30 0x7f6963835c25 in NodeKafka::Workers::ConnectionMetadata::HandleOKCallback() ../src/workers.cc:116 #31 0x7f69637b6bac in Nan::AsyncWorker::WorkComplete() ../node_modules/nan/nan.h:2008 #32 0x7f69637b6bac in Nan::AsyncExecuteComplete(uv_work_s*) ../node_modules/nan/nan.h:2365 #33 0x7f69637b6bac in Nan::AsyncExecuteComplete(uv_work_s*, int) ../node_modules/nan/nan.h:2369 #34 0x18bb75c in uv__work_done ../deps/uv/src/threadpool.c:329 Thread T4 created by T0 here: #0 0x7f6c0e93b2a2 in __interceptor_pthread_create ../../../../src/libsanitizer/asan/asan_interceptors.cpp:214 #1 0x18cdf7b in uv_thread_create_ex ../deps/uv/src/unix/thread.c:172 #2 0x18cdf7b in uv_thread_create ../deps/uv/src/unix/thread.c:126 #3 0xd4c115 in node::WorkerThreadsTaskRunner::WorkerThreadsTaskRunner(int) (/usr/bin/node+0xd4c115) #4 0xd4c3cb in node::NodePlatform::NodePlatform(int, v8::TracingController*, v8::PageAllocator*) (/usr/bin/node+0xd4c3cb) #5 0xc6f785 in node::InitializeOncePerProcessInternal(std::vector, std::allocator >, std::allocator, std::allocator > > > const&, node::ProcessInitializationFlags::Flags) (/usr/bin/node+0xc6f785) #6 0xc70c83 in node::Start(int, char**) (/usr/bin/node+0xc70c83) #7 0x7f6c0e3d7d09 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x23d09) SUMMARY: AddressSanitizer: alloc-dealloc-mismatch ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:123 in __interceptor_free ``` Reproducer: ```js const { Producer } = require('.'); var producer = new Producer({ 'client.id': 'kafka-test', 'metadata.broker.list': '127.0.0.1:9092', 'dr_cb': true, 'debug': 'all' }); producer.connect({}, function(err) { if (err) { console.error('Error connecting to Kafka:', err); return; } producer.produce('test', null, Buffer.from(''), ''); producer.disconnect(); }); ``` There should not be a need to allocate a buffer in this scenario. Use a single static character array holding a null-terminator as the payload/key instead. --- src/producer.cc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/producer.cc b/src/producer.cc index 8e20320a..4a778c04 100644 --- a/src/producer.cc +++ b/src/producer.cc @@ -7,6 +7,7 @@ * of the MIT license. See the LICENSE.txt file for details. */ +#include #include #include @@ -474,6 +475,7 @@ NAN_METHOD(Producer::NodeProduce) { size_t message_buffer_length; void* message_buffer_data; + static std::array empty_string{'\0'}; if (info[2]->IsNull()) { // This is okay for whatever reason @@ -497,9 +499,8 @@ NAN_METHOD(Producer::NodeProduce) { message_buffer_data = node::Buffer::Data(message_buffer_object); if (message_buffer_data == NULL) { // empty string message buffer should not end up as null message - v8::Local message_buffer_object_emptystring = Nan::NewBuffer(new char[0], 0).ToLocalChecked(); - message_buffer_length = node::Buffer::Length(message_buffer_object_emptystring); - message_buffer_data = node::Buffer::Data(message_buffer_object_emptystring); + message_buffer_length = 0; + message_buffer_data = reinterpret_cast(empty_string.data()); } } @@ -527,9 +528,8 @@ NAN_METHOD(Producer::NodeProduce) { key_buffer_data = node::Buffer::Data(key_buffer_object); if (key_buffer_data == NULL) { // empty string key buffer should not end up as null key - v8::Local key_buffer_object_emptystring = Nan::NewBuffer(new char[0], 0).ToLocalChecked(); - key_buffer_length = node::Buffer::Length(key_buffer_object_emptystring); - key_buffer_data = node::Buffer::Data(key_buffer_object_emptystring); + key_buffer_length = 0; + key_buffer_data = reinterpret_cast(empty_string.data()); } } else { // If it was a string just use the utf8 value.