From 247931f2dfc46ca72c4fc27cf519d7d656e979ef Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Tue, 19 Aug 2025 14:52:06 +0200 Subject: [PATCH 1/2] logging/Manager: Non-null strings for empty strings After #4724, empty strings would result in nullptrs being stored in the threading::Value's string_val.data field instead of a valid pointer to an empty strings. This upsets UBSAN's nonnull check for memcpy() [01:29:45.807] ../../src/SerializationFormat.cc:80:33: runtime error: null pointer passed as argument 2, which is declared to never be null [01:29:45.807] /usr/include/string.h:44:28: note: nonnull attribute specified here [01:29:45.807] #0 0x5b2e9c933a3f in zeek::detail::SerializationFormat::WriteData(void const*, unsigned long) /zeek/build/src/../../src/SerializationFormat.cc:80:5 [01:29:45.807] #1 0x5b2e9c935184 in zeek::detail::BinarySerializationFormat::Write(char const*, int, char const*) /zeek/build/src/../../src/SerializationFormat.cc:371:40 Continue to allocate the empty string for now as a fix. --- src/logging/Manager.cc | 3 --- src/threading/SerialTypes.cc | 5 ++++- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/logging/Manager.cc b/src/logging/Manager.cc index ee704a6b9d..84e69b6d7b 100644 --- a/src/logging/Manager.cc +++ b/src/logging/Manager.cc @@ -1511,9 +1511,6 @@ threading::Value Manager::ValToLogVal(WriterInfo* info, const Stream* stream, st info->total_truncated_string_fields->Inc(); } - if ( allowed_bytes == 0 ) - return lval; - char* buf = new char[allowed_bytes]; memcpy(buf, s->Bytes(), allowed_bytes); diff --git a/src/threading/SerialTypes.cc b/src/threading/SerialTypes.cc index 294d4e78a9..433136e475 100644 --- a/src/threading/SerialTypes.cc +++ b/src/threading/SerialTypes.cc @@ -376,7 +376,10 @@ bool Value::Write(detail::SerializationFormat* fmt) const { case TYPE_ENUM: case TYPE_STRING: case TYPE_FILE: - case TYPE_FUNC: return fmt->Write(val.string_val.data, val.string_val.length, "string"); + case TYPE_FUNC: { + assert(val.string_val.data); + return fmt->Write(val.string_val.data, val.string_val.length, "string"); + } case TYPE_TABLE: { if ( ! fmt->Write(val.set_val.size, "set_size") ) From c44ce78591a54beef8c6daddb32b2e0f602dd7d7 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Tue, 19 Aug 2025 20:00:21 +0200 Subject: [PATCH 2/2] logging/Manager: Also pass non-null vector and set Primarily to align with strings and also to keep the plugin API the same. --- src/logging/Manager.cc | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/logging/Manager.cc b/src/logging/Manager.cc index 84e69b6d7b..da1ff9d192 100644 --- a/src/logging/Manager.cc +++ b/src/logging/Manager.cc @@ -1566,9 +1566,6 @@ threading::Value Manager::ValToLogVal(WriterInfo* info, const Stream* stream, st info->total_truncated_containers->Inc(); } - if ( allowed_elements == 0 ) - return lval; - lval.val.set_val.vals = new threading::Value*[allowed_elements]; for ( size_t i = 0; i < allowed_elements && total_record_size < max_log_record_size; i++ ) { @@ -1597,9 +1594,6 @@ threading::Value Manager::ValToLogVal(WriterInfo* info, const Stream* stream, st info->total_truncated_containers->Inc(); } - if ( allowed_elements == 0 ) - return lval; - lval.val.vector_val.vals = new threading::Value*[allowed_elements]; auto& vv = vec->RawVec();