Bug#50373 --secure-file-priv=""

Iterative patch improvement. Previously committed patch
caused wrong result on Windows. The previous patch also
broke secure_file_priv for symlinks since not all file
paths which must be compared against this variable are
normalized using the same norm.

The server variable opt_secure_file_priv wasn't
normalized properly and caused the operations
LOAD DATA INFILE .. INTO TABLE ..
and
SELECT load_file(..)
to do different interpretations of the 
--secure-file-priv option.
     
The patch moves code to the server initialization
routines so that the path always is normalized
once and only once.
      
It was also intended that setting the option
to an empty string should be equal to 
lifting all previously set restrictions. This
is also fixed by this patch.

mysql-test/r/loaddata.result

@@ -202,12 +202,6 @@ select * from t1;
 a	b	c
 10	NULL	Ten
 15	NULL	Fifteen
-show variables like "secure_file_pri%";
-Variable_name	Value
-secure_file_priv	MYSQLTEST_VARDIR
-select @@secure_file_priv;
-@@secure_file_priv
-MYSQLTEST_VARDIR
 set @@secure_file_priv= 0;
 ERROR HY000: Variable 'secure_file_priv' is a read only variable
 truncate table t1;

mysql-test/t/loaddata.test

@@ -153,10 +153,16 @@ select * from t1;
 #
 # It should not be possible to load from a file outside of vardir
 
---replace_result $MYSQLTEST_VARDIR MYSQLTEST_VARDIR
-show variables like "secure_file_pri%";
---replace_result $MYSQLTEST_VARDIR MYSQLTEST_VARDIR
-select @@secure_file_priv;
+## The following lines were disabled because of patch for
+## bug 50373. MYSQLTEST_VARDIR doesn't rewrite symlinks
+## to real paths, but this is done for secure_file_priv.
+## Because of this the result can't be replaced if the
+## test suite runs with the --mem option which creates
+## symlinks to the ramdisk.
+#--replace_result $MYSQLTEST_VARDIR MYSQLTEST_VARDIR
+#show variables like "secure_file_pri%";
+#--replace_result $MYSQLTEST_VARDIR MYSQLTEST_VARDIR
+#select @@secure_file_priv;
 --error 1238
 set @@secure_file_priv= 0;
 

sql/item_strfunc.cc

@@ -2959,8 +2959,7 @@ String *Item_load_file::val_str(String *str)
 		   MY_RELATIVE_PATH | MY_UNPACK_FILENAME);
 
   /* Read only allowed from within dir specified by secure_file_priv */
-  if (opt_secure_file_priv &&
-      strncmp(opt_secure_file_priv, path, strlen(opt_secure_file_priv)))
+  if (!is_secure_file_path(path))
     goto err;
 
   if (!my_stat(path, &stat_info, MYF(0)))

sql/mysql_priv.h

@@ -1832,6 +1832,12 @@ void sql_perror(const char *message);
 
 bool fn_format_relative_to_data_home(char * to, const char *name,
 				     const char *dir, const char *extension);
+/**
+  Test a file path to determine if the path is compatible with the secure file
+  path restriction.
+*/
+bool is_secure_file_path(char *path);
+
 #ifdef MYSQL_SERVER
 File open_binlog(IO_CACHE *log, const char *log_file_name,
                  const char **errmsg);

sql/mysqld.cc

@@ -8776,6 +8776,45 @@ fn_format_relative_to_data_home(char * to, const char *name,
 }
 
 
+/**
+  Test a file path to determine if the path is compatible with the secure file
+  path restriction.
+ 
+  @param path null terminated character string
+
+  @return
+    @retval TRUE The path is secure
+    @retval FALSE The path isn't secure
+*/
+
+bool is_secure_file_path(char *path)
+{
+  char buff1[FN_REFLEN], buff2[FN_REFLEN];
+  /*
+    All paths are secure if opt_secure_file_path is 0
+  */
+  if (!opt_secure_file_priv)
+    return TRUE;
+
+  if (my_realpath(buff1, path, 0))
+  {
+    /*
+      The supplied file path might have been a file and not a directory.
+    */
+    int length= (int)dirname_length(path);
+    if (length >= FN_REFLEN)
+      return FALSE;
+    memcpy(buff2, path, length);
+    buff2[length]= '\0';
+    if (length == 0 || my_realpath(buff1, buff2, 0))
+      return FALSE;
+  }
+  convert_dirname(buff2, buff1, NullS);
+  if (strncmp(opt_secure_file_priv, buff2, strlen(opt_secure_file_priv)))
+    return FALSE;
+  return TRUE;
+}
+
 static int fix_paths(void)
 {
   char buff[FN_REFLEN],*pos;
@@ -8843,14 +8882,13 @@ static int fix_paths(void)
     }
     else
     {
-      convert_dirname(buff, opt_secure_file_priv, NullS);
-      char *secure_file_real_path= (char *)my_malloc(FN_REFLEN, MYF(MY_FAE));
-      if (secure_file_real_path == 0 ||
-          my_realpath(secure_file_real_path, buff, 0))
+      if (my_realpath(buff, opt_secure_file_priv, 0))
       {
         sql_print_warning("Failed to normalize the argument for --secure-file-priv.");
         return 1;
       }
+      char *secure_file_real_path= (char *)my_malloc(FN_REFLEN, MYF(MY_FAE));
+      convert_dirname(secure_file_real_path, buff, NullS);
       my_free(opt_secure_file_priv, MYF(0));
       opt_secure_file_priv= secure_file_real_path;
     }

sql/sql_class.cc

@@ -1831,8 +1831,7 @@ static File create_file(THD *thd, char *path, sql_exchange *exchange,
   else
     (void) fn_format(path, exchange->file_name, mysql_real_data_home, "", option);
 
-  if (opt_secure_file_priv &&
-      strncmp(opt_secure_file_priv, path, strlen(opt_secure_file_priv)))
+  if (!is_secure_file_path(path))
   {
     /* Write only allowed to dir or subdir specified by secure_file_priv */
     my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--secure-file-priv");

sql/sql_load.cc

@@ -348,14 +348,11 @@ int mysql_load(THD *thd,sql_exchange *ex,TABLE_LIST *table_list,
         DBUG_ASSERT(FALSE); 
 #endif
       }
-      else if (opt_secure_file_priv)
+      else if (!is_secure_file_path(name))
       {
-        if (strncmp(opt_secure_file_priv, name, strlen(opt_secure_file_priv)))
-        {
-          /* Read only allowed from within dir specified by secure_file_priv */
-          my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--secure-file-priv");
-          DBUG_RETURN(TRUE);
-        }
+        /* Read only allowed from within dir specified by secure_file_priv */
+        my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--secure-file-priv");
+        DBUG_RETURN(TRUE);
       }
 
     }