From 8fed26824b592b3b55cb34f8bca3e197d2e2e568 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Fri, 14 Feb 2020 21:26:55 -0800 Subject: [PATCH] Replace va_list fmt() overload with vfmt() Using an overload that takes a va_list argument potentially causes accidental misuse on platforms (e.g. 32-bit) where va_list is implemented as a type that may collide with commonly-used argument types. For example: char* c = copy_string("hi"); fmt("%s", (const char*)c); fmt("%s", c); The first fmt() call correctly goes through fmt(const char*, ...) first, but the second mistakenly goes through fmt(const char*, va_list) first because variadic function overloads have lower priority during overload resolution and va_list on a 32-bit system happens to be defined as a pointer type that can match with "char*" but not "const char*". --- NEWS | 6 ++++++ src/broker/Manager.cc | 2 +- src/supervisor/Supervisor.cc | 2 +- src/util.cc | 4 ++-- src/util.h | 2 +- 5 files changed, 11 insertions(+), 5 deletions(-) diff --git a/NEWS b/NEWS index d3777ae5f7..8bdffeda9b 100644 --- a/NEWS +++ b/NEWS @@ -15,6 +15,12 @@ Changed Functionality Removed Functionality --------------------- +- The fmt() function which takes a va_list argument is replaced, use + the new vfmt() function for equivalent functionality. The former is + deprecated because overloading it with the variadic fmt() function + can cause the unintended overload to be chosen depending on how the + platform implements va_list. + Deprecated Functionality ------------------------ diff --git a/src/broker/Manager.cc b/src/broker/Manager.cc index eae0ce723f..bb5d3a6af3 100644 --- a/src/broker/Manager.cc +++ b/src/broker/Manager.cc @@ -625,7 +625,7 @@ void Manager::Error(const char* format, ...) { va_list args; va_start(args, format); - auto msg = fmt(format, args); + auto msg = vfmt(format, args); va_end(args); if ( script_scope ) diff --git a/src/supervisor/Supervisor.cc b/src/supervisor/Supervisor.cc index a1dd2a21d4..685b0707e7 100644 --- a/src/supervisor/Supervisor.cc +++ b/src/supervisor/Supervisor.cc @@ -724,7 +724,7 @@ void Stem::ReportStatus(const Supervisor::Node& node) const void Stem::Log(std::string_view type, const char* format, va_list args) const { - auto raw_msg = fmt(format, args); + auto raw_msg = vfmt(format, args); if ( getenv("ZEEK_DEBUG_STEM_STDERR") ) { diff --git a/src/util.cc b/src/util.cc index 6429e1481d..e75e79882d 100644 --- a/src/util.cc +++ b/src/util.cc @@ -817,7 +817,7 @@ const char* fmt_bytes(const char* data, int len) return buf; } -const char* fmt(const char* format, va_list al) +const char* vfmt(const char* format, va_list al) { static char* buf = 0; static unsigned int buf_len = 1024; @@ -848,7 +848,7 @@ const char* fmt(const char* format, ...) { va_list al; va_start(al, format); - auto rval = fmt(format, al); + auto rval = vfmt(format, al); va_end(al); return rval; } diff --git a/src/util.h b/src/util.h index cb5e1fc2eb..ae2f0fc2d4 100644 --- a/src/util.h +++ b/src/util.h @@ -188,7 +188,7 @@ extern std::string strtolower(const std::string& s); extern const char* fmt_bytes(const char* data, int len); // Note: returns a pointer into a shared buffer. -extern const char* fmt(const char* format, va_list args); +extern const char* vfmt(const char* format, va_list args); // Note: returns a pointer into a shared buffer. extern const char* fmt(const char* format, ...) __attribute__((format (printf, 1, 2)));