Bug#24388753: PRIVILEGE ESCALATION USING MYSQLD_SAFE

[This is the 5.5/5.6 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

@@ -2293,6 +2293,77 @@ bool MYSQL_LOG::init_and_set_log_file_name(const char *log_name,
 }
 
 
+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.
+
+  Use realpath() to get the path with symbolic links
+  expanded. Then, close the file, and reopen the real path using the
+  O_NOFOLLOW flag. This will reject following 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.
+
+  @retval file descriptor to open file with 'real_file_name', or '-1'
+          in case of errors.
+*/
+
+#ifndef _WIN32
+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)
+{
+  DBUG_ASSERT(file);
+  DBUG_ASSERT(opened_file_name);
+
+  /* Buffer for realpath must have capacity for PATH_MAX. */
+  char real_file_name[PATH_MAX];
+
+  /* Get realpath, validate, 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;
+
+  if (strlen(real_file_name) > FN_REFLEN)
+    return -1;
+
+  if (!is_valid_log_name(real_file_name, strlen(real_file_name)))
+  {
+    sql_print_error("Invalid log file name after expanding symlinks: '%s'",
+                    real_file_name);
+    return -1;
+  }
+
+  return mysql_file_open(log_file_key, real_file_name,
+                         open_flags | O_NOFOLLOW,
+                         MYF(MY_WME | ME_WAITTANG));
+}
+#endif // _WIN32
+
 /*
   Open a (new) log file.
 
@@ -2358,8 +2429,22 @@ bool MYSQL_LOG::open(
 
   if ((file= mysql_file_open(log_file_key,
                              log_file_name, open_flags,
-                             MYF(MY_WME | ME_WAITTANG))) < 0 ||
-      init_io_cache(&log_file, file, IO_SIZE, io_cache_type,
+                             MYF(MY_WME | ME_WAITTANG))) < 0)
+    goto err;
+
+#ifndef _WIN32
+  /* Reopen and validate path. */
+  if ((log_type_arg == LOG_UNKNOWN || log_type_arg == LOG_NORMAL) &&
+      (file= mysql_file_real_name_reopen(file,
+#ifdef HAVE_PSI_INTERFACE
+                                         log_file_key,
+#endif
+                                         open_flags,
+                                         log_file_name)) < 0)
+    goto err;
+#endif // _WIN32
+
+  if (init_io_cache(&log_file, file, IO_SIZE, io_cache_type,
                     mysql_file_tell(file, MYF(MY_WME)), 0,
                     MYF(MY_WME | MY_NABP |
                         ((log_type == LOG_BIN) ? MY_WAIT_IF_FULL : 0))))

sql/log.h

@@ -717,6 +717,16 @@ File open_binlog(IO_CACHE *log, const char *log_file_name,
 
 char *make_log_name(char *buff, const char *name, const char* log_ext);
 
+/**
+  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);
+
 extern MYSQL_PLUGIN_IMPORT MYSQL_BIN_LOG mysql_bin_log;
 extern LOGGER logger;
 

sql/mysqld.cc

@@ -1,4 +1,4 @@
-/* Copyright (c) 2000, 2015, Oracle and/or its affiliates. All rights
+/* Copyright (c) 2000, 2016, Oracle and/or its affiliates. All rights
    reserved.
 
    This program is free software; you can redistribute it and/or modify
@@ -3512,6 +3512,22 @@ static int init_common_variables()
                       "--log-slow-queries option, log tables are used. "
                       "To enable logging to files use the --log-output=file option.");
 
+  if (opt_logname &&
+      !is_valid_log_name(opt_logname, strlen(opt_logname)))
+  {
+    sql_print_error("Invalid value for --general_log_file: %s",
+                    opt_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)                                            \
   {                                                             \

sql/sys_vars.cc

@@ -1,4 +1,4 @@
-/* Copyright (c) 2002, 2015, Oracle and/or its affiliates. All rights reserved.
+/* Copyright (c) 2002, 2016, Oracle and/or its affiliates. All rights reserved.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -2810,6 +2810,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);
@@ -2856,7 +2864,7 @@ static bool check_log_path(sys_var *self, THD *thd, set_var *var)
   return false;
 }
 static bool fix_log(char** logname, const char* default_logname,
-                    const char*ext, bool enabled, void (*reopen)(char*))
+                    const char*ext, bool enabled, bool (*reopen)(char*))
 {
   if (!*logname) // SET ... = DEFAULT
   {
@@ -2868,16 +2876,17 @@ static bool fix_log(char** logname, const char* default_logname,
   }
   logger.lock_exclusive();
   mysql_mutex_unlock(&LOCK_global_system_variables);
+  bool error= false;
   if (enabled)
-    reopen(*logname);
+    error= reopen(*logname);
   logger.unlock();
   mysql_mutex_lock(&LOCK_global_system_variables);
-  return false;
+  return error;
 }
-static void reopen_general_log(char* name)
+static bool reopen_general_log(char* name)
 {
   logger.get_log_file_handler()->close(0);
-  logger.get_log_file_handler()->open_query_log(name);
+  return logger.get_log_file_handler()->open_query_log(name);
 }
 static bool fix_general_log_file(sys_var *self, THD *thd, enum_var_type type)
 {
@@ -2890,10 +2899,10 @@ static Sys_var_charptr Sys_general_log_path(
        IN_FS_CHARSET, DEFAULT(0), NO_MUTEX_GUARD, NOT_IN_BINLOG,
        ON_CHECK(check_log_path), ON_UPDATE(fix_general_log_file));
 
-static void reopen_slow_log(char* name)
+static bool reopen_slow_log(char* name)
 {
   logger.get_slow_log_file_handler()->close(0);
-  logger.get_slow_log_file_handler()->open_slow_log(name);
+  return logger.get_slow_log_file_handler()->open_slow_log(name);
 }
 static bool fix_slow_log_file(sys_var *self, THD *thd, enum_var_type type)
 {