Bug#26585560 - MYSQL DAEMON SHOULD CREATE ITS PID FILE AS

ROOT

DESCRIPTION
===========
If the .pid file is created at a world-writable location,
it can be compromised by replacing the server's pid with
another running server's (or some other non-mysql process)
PID causing abnormal behaviour.

ANALYSIS
========
In such a case, user should be warned that .pid file is
being created at a world-writable location.

FIX
===
A new function is_file_or_dir_world_writable() is defined
and it is called in create_pid_file() before .pid file
creation. If the location is world-writable, a relevant
warning is thrown.

NOTE
====
1. PID file is always created with permission bit 0664, so
for outside world its read-only.
2. Ignoring the case when permission is denied to get the
dir stats since the .pid file creation would fail anyway in
such a case.

include/sql_common.h

@@ -107,6 +107,7 @@ void mysql_client_plugin_deinit();
 struct st_mysql_client_plugin;
 extern struct st_mysql_client_plugin *mysql_client_builtins[];
 extern my_bool libmysql_cleartext_plugin_enabled;
+int is_file_or_dir_world_writable(const char *filepath);
 
 #ifdef	__cplusplus
 }

mysql-test/include/mtr_warnings.sql

@@ -1,4 +1,4 @@
--- Copyright (c) 2008, 2016, Oracle and/or its affiliates. All rights reserved.
+-- Copyright (c) 2008, 2017, 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
@@ -209,6 +209,12 @@ INSERT INTO global_suppressions VALUES
  */
  ("Insecure configuration for --secure-file-priv:*"),
 
+ /*
+  Bug#26585560, warning related to --pid-file
+ */
+ ("Insecure configuration for --pid-file:*"),
+ ("Few location(s) are inaccessible while checking PID filepath"),
+
  ("THE_LAST_SUPPRESSION")||
 
 

sql-common/my_path_permissions.cc

@@ -0,0 +1,54 @@
+/* Copyright (c) 2017, 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 the
+   Free Software Foundation; version 2 of the License.
+
+   This program is distributed in the hope that it will be useful, but
+   WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+   02110-1301, USA */
+
+#include "my_dir.h"
+
+#ifdef  __cplusplus
+extern "C" {
+#endif
+
+/**
+  Check if a file/dir is world-writable (only on non-Windows platforms)
+
+  @param [in] Path of the file/dir to be checked
+
+  @returns Status of the file/dir check
+    @retval -2 Permission denied to check attributes of file/dir
+    @retval -1 Error in reading file/dir
+    @retval  0 File/dir is not world-writable
+    @retval  1 File/dir is world-writable
+ */
+
+int is_file_or_dir_world_writable(const char *path)
+{
+  MY_STAT stat_info;
+  (void)path; // avoid unused param warning when built on Windows
+#ifndef _WIN32
+  if (!my_stat(path, &stat_info, MYF(0)))
+  {
+      return (errno == EACCES) ? -2 : -1;
+  }
+  if ((stat_info.st_mode & S_IWOTH) &&
+      ((stat_info.st_mode & S_IFMT) == S_IFREG ||   /* file   */
+       (stat_info.st_mode & S_IFMT) == S_IFDIR))    /* or dir */
+    return 1;
+#endif
+  return 0;
+}
+
+#ifdef  __cplusplus
+}
+#endif

sql/CMakeLists.txt

@@ -78,7 +78,7 @@ SET (SQL_SOURCE
                sql_profile.cc event_parse_data.cc sql_alter.cc
                sql_signal.cc rpl_handler.cc mdl.cc sql_admin.cc
                transaction.cc sys_vars.cc sql_truncate.cc datadict.cc
-               sql_reload.cc
+               sql_reload.cc ../sql-common/my_path_permissions.cc
                ${GEN_SOURCES}
                ${CONF_SOURCES}
                ${MYSYS_LIBWRAP_SOURCE})

sql/mysqld.cc

@@ -1,4 +1,4 @@
-/* Copyright (c) 2000, 2016, Oracle and/or its affiliates. All rights
+/* Copyright (c) 2000, 2017, Oracle and/or its affiliates. All rights
    reserved.
 
    This program is free software; you can redistribute it and/or modify
@@ -7996,6 +7996,40 @@ static int test_if_case_insensitive(const char *dir_name)
 static void create_pid_file()
 {
   File file;
+  bool check_parent_path= 1, is_path_accessible= 1;
+  char pid_filepath[FN_REFLEN], *pos= NULL;
+  /* Copy pid file name to get pid file path */
+  strcpy(pid_filepath, pidfile_name);
+
+  /* Iterate through the entire path to check if even one of the sub-dirs
+     is world-writable */
+  while (check_parent_path && (pos= strrchr(pid_filepath, FN_LIBCHAR))
+         && (pos != pid_filepath)) /* shouldn't check root */
+  {
+    *pos= '\0';  /* Trim the inner-most dir */
+    switch (is_file_or_dir_world_writable(pid_filepath))
+    {
+      case -2:
+        is_path_accessible= 0;
+        break;
+      case -1:
+        sql_perror("Can't start server: can't check PID filepath");
+        exit(1);
+      case 1:
+        sql_print_warning("Insecure configuration for --pid-file: Location "
+                          "'%s' in the path is accessible to all OS users. "
+                          "Consider choosing a different directory.",
+                          pid_filepath);
+        check_parent_path= 0;
+        break;
+      case 0:
+        continue; /* Keep checking the parent dir */
+    }
+  }
+  if (!is_path_accessible)
+  {
+    sql_print_warning("Few location(s) are inaccessible while checking PID filepath.");
+  }
   if ((file= mysql_file_create(key_file_pid, pidfile_name, 0664,
                                O_WRONLY | O_TRUNC, MYF(MY_WME))) >= 0)
   {