Bug#32890918: Writing the Windows EventLog would occasionally fail

Tatiana Azundris Nuernberg · dahlerlend · commit 56e66fa1eb16 · 2021-06-21T15:48:11.000+02:00
When converting the message to be logged to UCS-2 for Windows, the
conversion routine could read beyond the end of the input string.
Patch prevents this, adds comments to better explain the idiosyncrasies
of our Windows logging, and renames variables to hopefully prevent
uncertainty in the future.

This patch comes with a big thank you to Shane Bester for dumps
and other helpful files when the issue was elusive on my machine,
and to Daniel Blanchard who went above and beyond creating a much
optimized proof once I had a patch.

Change-Id: Id28d09dcc5a51caf67d966034d485d28eed5c84e
diff --git a/mysys/my_syslog.cc b/mysys/my_syslog.cc
@@ -52,7 +52,31 @@ extern CHARSET_INFO my_charset_utf16le_bin;
 #endif
 
 #ifdef _WIN32
+/**
+  Windows EventLog messages
+
+  See sql/message.mc for the defined messages on Windows
+  (and on how to add/modify messages).
+
+  Presently, only one message is used: message 100 (MSG_DEFAULT)
+  is a template "%1" into which we insert the message my_syslog()
+  is given.
+
+  0x[C][000][0064] -- 32 bits, left to right:
+
+  0xC:
+     2:Severity         (0x3    == Error)          (0xC>>2)
+     1:Custom or System (0x0    == System)         ((0xC>>1) & 1)
+     1:Rsrvd            (0x0)                      (0xC & 1)
+
+  0x000:
+    12:Facility         (0x000  == FACILITY_NULL)
+
+  0x0064
+    16:Code             (0x0064 == 100)
+*/
 #define MSG_DEFAULT 0xC0000064L
+
 static HANDLE hEventLog = NULL;  // global
 #endif
 
@@ -63,7 +87,7 @@ static HANDLE hEventLog = NULL;  // global
 
   @param cs                   Character set info of the message string
   @param level                Log level
-  @param msg                  Message to be logged
+  @param msg                  Message to be logged (C-string)
 
   @return
      0 Success
@@ -75,14 +99,21 @@ int my_syslog(const CHARSET_INFO *cs MY_ATTRIBUTE((unused)),
   int _level = EVENTLOG_INFORMATION_TYPE;
   wchar_t buff[MAX_SYSLOG_MESSAGE_SIZE];
   wchar_t *u16buf = NULL;
-  size_t nchars;
-  uint dummy_errors;
+  size_t msg_len;     // bytes (not wide-chars) in input
+  size_t nbytes;      // bytes (not wide-chars) in output
+  uint dummy_errors;  // number of conversion errors
 
   DBUG_TRACE;
 
+  if ((msg == nullptr) || ((msg_len = strlen(msg)) == 0)) return 0;
+
+  // limit input to buffer size
+  if (msg_len >= MAX_SYSLOG_MESSAGE_SIZE) msg_len = MAX_SYSLOG_MESSAGE_SIZE - 1;
+
   switch (level) {
-    case INFORMATION_LEVEL:
     case SYSTEM_LEVEL:
+      // fall-through. consider EVENTLOG_SUCCESS.
+    case INFORMATION_LEVEL:
       _level = EVENTLOG_INFORMATION_TYPE;
       break;
     case WARNING_LEVEL:
@@ -96,16 +127,24 @@ int my_syslog(const CHARSET_INFO *cs MY_ATTRIBUTE((unused)),
   }
 
   if (hEventLog) {
-    nchars = my_convert((char *)buff, sizeof(buff) - sizeof(buff[0]),
-                        &my_charset_utf16le_bin, msg, MAX_SYSLOG_MESSAGE_SIZE,
-                        cs, &dummy_errors);
+    nbytes =
+        my_convert((char *)buff, sizeof(buff) - sizeof(buff[0]),
+                   &my_charset_utf16le_bin, msg, msg_len, cs, &dummy_errors);
 
     // terminate it with NULL
-    buff[nchars / sizeof(wchar_t)] = L'\0';
+    buff[nbytes / sizeof(wchar_t)] = L'\0';
     u16buf = buff;
 
-    if (!ReportEventW(hEventLog, _level, 0, MSG_DEFAULT, NULL, 1, 0,
-                      (LPCWSTR *)&u16buf, NULL))
+    // Fetch mysqld's Windows default message, insert u16buf, log the result.
+    if (!ReportEventW(hEventLog,
+                      _level,              // severity
+                      0,                   // app-defined event category
+                      MSG_DEFAULT,         // event ID / message ID (see above)
+                      NULL,                // security identifier
+                      1,                   // number of strings in u16buf
+                      0,                   // number of bytes in raw data
+                      (LPCWSTR *)&u16buf,  // 0-terminated strings
+                      NULL))               // raw (binary data)
       goto err;
   }
 
@@ -122,6 +161,8 @@ int my_syslog(const CHARSET_INFO *cs MY_ATTRIBUTE((unused)),
 
   DBUG_TRACE;
 
+  if ((msg == nullptr) || (*msg == '\0')) return 0;
+
   switch (level) {
     case INFORMATION_LEVEL:
     case SYSTEM_LEVEL:
diff --git a/sql/server_component/log_builtins.cc b/sql/server_component/log_builtins.cc
@@ -66,7 +66,6 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301  USA */
 
 extern CHARSET_INFO my_charset_utf16le_bin;  // used in Windows EventLog
 static HANDLE hEventLog = NULL;              // global
-#define MSG_DEFAULT 0xC0000064L
 #endif
 
 PSI_memory_key key_memory_log_error_loaded_services;
