Bug#24388753: PRIVILEGE ESCALATION USING MYSQLD_SAFE

[This is the 5.7/8.0 version of the bugfix].

The problem was that it was possible to write log files ending
in .ini/.cnf that later could be parsed as an options file.
This made it possible for users to specify startup options
without the permissions to do so.

This patch fixes the problem by disallowing general query log
and slow query log to be written to files ending in .ini and .cnf.

Author: ssorumgard

sql/log.cc

@@ -519,6 +519,100 @@ static int make_iso8601_timestamp(char *buf, ulonglong utime= 0)
 }
 
 
+bool is_valid_log_name(const char *name, size_t len)
+{
+  if (len > 3)
+  {
+    const char *tail= name + len - 4;
+    if (my_strcasecmp(system_charset_info, tail, ".ini") == 0 ||
+        my_strcasecmp(system_charset_info, tail, ".cnf") == 0)
+    {
+      return false;
+    }
+  }
+  return true;
+}
+
+
+/**
+  Get the real log file name, and possibly reopen file.
+
+  The implementation is platform dependent due to differences in how this is
+  supported:
+
+  On Windows, we get the actual path based on the file descriptor. This path is
+  copied into the supplied buffer. The 'file' parameter is returned without
+  re-opening.
+
+  On other platforms, we use realpath() to get the path with symbolic links
+  expanded. Then, we close the file, and reopen the real path using the
+  O_NOFOLLOW flag. This will reject folowing symbolic links.
+
+  @param          file                  File descriptor.
+  @param          log_file_key          Key for P_S instrumentation.
+  @param          open_flags            Flags to use for opening the file.
+  @param          opened_file_name      Name of the open fd.
+  @param [out]    real_file_name        Buffer for actual name of the fd.
+
+  @retval file descriptor to open file with 'real_file_name', or '-1'
+          in case of errors.
+*/
+
+static File mysql_file_real_name_reopen(File file,
+#ifdef HAVE_PSI_INTERFACE
+                                        PSI_file_key log_file_key,
+#endif
+                                        int open_flags,
+                                        const char *opened_file_name,
+                                        char *real_file_name)
+{
+  DBUG_ASSERT(file);
+  DBUG_ASSERT(opened_file_name);
+  DBUG_ASSERT(real_file_name);
+
+#ifdef _WIN32
+  /* On Windows, O_NOFOLLOW is not supported. Verify real path from fd. */
+  DWORD real_length= GetFinalPathNameByHandle(my_get_osfhandle(file),
+                                              real_file_name,
+                                              FN_REFLEN,
+                                              FILE_NAME_OPENED);
+
+  /* May ret 0 if e.g. on a ramdisk. Ignore - return open file and name. */
+  if (real_length == 0)
+  {
+    strcpy(real_file_name, opened_file_name);
+    return file;
+  }
+
+  if (real_length > FN_REFLEN)
+  {
+    mysql_file_close(file, MYF(0));
+    return -1;
+  }
+
+  return file;
+#else
+  /* On *nix, get realpath, open realpath with O_NOFOLLOW. */
+  if (realpath(opened_file_name, real_file_name) == NULL)
+  {
+    (void) mysql_file_close(file, MYF(0));
+    return -1;
+  }
+
+  if (mysql_file_close(file, MYF(0)))
+    return -1;
+
+  /* Make sure the real path is not too long. */
+  if (strlen(real_file_name) > FN_REFLEN)
+    return -1;
+
+  return mysql_file_open(log_file_key, real_file_name,
+                         open_flags | O_NOFOLLOW,
+                         MYF(MY_WME));
+#endif //_WIN32
+}
+
+
 bool File_query_log::open()
 {
   File file= -1;
@@ -552,12 +646,36 @@ bool File_query_log::open()
 
   db[0]= 0;
 
+  /* First, open the file to make sure it exists. */
   if ((file= mysql_file_open(m_log_file_key,
                              log_file_name,
                              O_CREAT | O_BINARY | O_WRONLY | O_APPEND,
                              MYF(MY_WME))) < 0)
     goto err;
 
+#ifdef _WIN32
+  char real_log_file_name[FN_REFLEN];
+#else
+  /* File name must have room for PATH_MAX. Checked against F_REFLEN later. */
+  char real_log_file_name[PATH_MAX];
+#endif // _Win32
+
+  /* Reopen and get real path. */
+  if ((file= mysql_file_real_name_reopen(file,
+#ifdef HAVE_PSI_INTERFACE
+                                         m_log_file_key,
+#endif
+                                         O_CREAT | O_BINARY | O_WRONLY | O_APPEND,
+                                         log_file_name, real_log_file_name)) < 0)
+    goto err;
+
+  if (!is_valid_log_name(real_log_file_name, strlen(real_log_file_name)))
+  {
+    sql_print_error("Invalid log file name after expanding symlinks: '%s'",
+                    real_log_file_name);
+    goto err;
+  }
+
   if ((pos= mysql_file_tell(file, MYF(MY_WME))) == MY_FILEPOS_ERROR)
   {
     if (my_errno() == ESPIPE)

sql/log.h

@@ -557,6 +557,16 @@ extern Query_logger query_logger;
 */
 char *make_query_log_name(char *buff, enum_log_table_type log_type);
 
+/**
+  Check given log name against certain blacklisted names/extensions.
+
+  @param name     Log name to check
+  @param len      Length of log name
+
+  @returns true if name is valid, false otherwise.
+*/
+bool is_valid_log_name(const char *name, size_t len);
+
 /**
   Check whether we need to write the current statement (or its rewritten
   version if it exists) to the slow query log.

sql/mysqld.cc

@@ -3022,6 +3022,22 @@ int init_common_variables()
                       "--slow-query-log-file option, log tables are used. "
                       "To enable logging to files use the --log-output=file option.");
 
+  if (opt_general_logname &&
+      !is_valid_log_name(opt_general_logname, strlen(opt_general_logname)))
+  {
+    sql_print_error("Invalid value for --general_log_file: %s",
+                    opt_general_logname);
+    return 1;
+  }
+
+  if (opt_slow_logname &&
+      !is_valid_log_name(opt_slow_logname, strlen(opt_slow_logname)))
+  {
+    sql_print_error("Invalid value for --slow_query_log_file: %s",
+                    opt_slow_logname);
+    return 1;
+  }
+
 #define FIX_LOG_VAR(VAR, ALT)                                   \
   if (!VAR || !*VAR)                                            \
     VAR= ALT;

sql/sys_vars.cc

@@ -4658,6 +4658,14 @@ static bool check_log_path(sys_var *self, THD *thd, set_var *var)
   if (!var->save_result.string_value.str)
     return true;
 
+  if (!is_valid_log_name(var->save_result.string_value.str,
+                         var->save_result.string_value.length))
+  {
+    my_error(ER_WRONG_VALUE_FOR_VAR, MYF(0),
+             self->name.str, var->save_result.string_value.str);
+    return true;
+  }
+
   if (var->save_result.string_value.length > FN_REFLEN)
   { // path is too long
     my_error(ER_PATH_LENGTH, MYF(0), self->name.str);