List:Commits« Previous MessageNext Message »
From:Kristofer Pettersson Date:May 3 2010 4:14pm
Subject:bzr commit into mysql-5.1-bugteam branch (kristofer.pettersson:3365)
Bug#50373
View as plain text  
#At file:///Users/thek/Development/mysql-5.1-bugteam/ based on revid:alexey.kopytov@stripped

 3365 Kristofer Pettersson	2010-05-03
      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
        * Removed test code which will currently break the much used --mem feature of mtr.
     @ mysql-test/t/loaddata.test
        * Removed test code which will currently break the much used --mem feature of mtr.
     @ sql/item_strfunc.cc
        * Replaced string comparing code on opt_secure_file_priv with an interface which guarantees that both file paths are normalized using the same norm on all platforms.
     @ sql/mysql_priv.h
        * Added signature for is_secure_file_path()
     @ sql/mysqld.cc
        * New function for checking if a path compatible with the secure path restriction.
        * Added initialization of the opt_secure_file_priv variable.
     @ sql/sql_class.cc
        * Replaced string comparing code on opt_secure_file_priv with an interface which guarantees that both file paths are normalized using the same norm on all platforms.
     @ sql/sql_load.cc
        * Replaced string comparing code on opt_secure_file_priv with an interface which guarantees that both file paths are normalized using the same norm on all platforms.

    modified:
      mysql-test/r/loaddata.result
      mysql-test/t/loaddata.test
      sql/item_strfunc.cc
      sql/mysql_priv.h
      sql/mysqld.cc
      sql/sql_class.cc
      sql/sql_load.cc
=== modified file 'mysql-test/r/loaddata.result'
--- a/mysql-test/r/loaddata.result	2010-04-20 14:17:34 +0000
+++ b/mysql-test/r/loaddata.result	2010-05-03 16:14:39 +0000
@@ -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;

=== modified file 'mysql-test/t/loaddata.test'
--- a/mysql-test/t/loaddata.test	2010-03-30 12:10:25 +0000
+++ b/mysql-test/t/loaddata.test	2010-05-03 16:14:39 +0000
@@ -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;
 

=== modified file 'sql/item_strfunc.cc'
--- a/sql/item_strfunc.cc	2010-04-02 20:30:22 +0000
+++ b/sql/item_strfunc.cc	2010-05-03 16:14:39 +0000
@@ -2959,8 +2959,7 @@ String *Item_load_file::val_str(String *
 		   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)))

=== modified file 'sql/mysql_priv.h'
--- a/sql/mysql_priv.h	2010-03-31 13:12:37 +0000
+++ b/sql/mysql_priv.h	2010-05-03 16:14:39 +0000
@@ -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);

=== modified file 'sql/mysqld.cc'
--- a/sql/mysqld.cc	2010-04-20 14:17:34 +0000
+++ b/sql/mysqld.cc	2010-05-03 16:14:39 +0000
@@ -8776,6 +8776,45 @@ fn_format_relative_to_data_home(char * t
 }
 
 
+/**
+  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;
     }

=== modified file 'sql/sql_class.cc'
--- a/sql/sql_class.cc	2010-04-14 09:53:59 +0000
+++ b/sql/sql_class.cc	2010-05-03 16:14:39 +0000
@@ -1831,8 +1831,7 @@ static File create_file(THD *thd, char *
   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");

=== modified file 'sql/sql_load.cc'
--- a/sql/sql_load.cc	2010-04-16 14:56:16 +0000
+++ b/sql/sql_load.cc	2010-05-03 16:14:39 +0000
@@ -348,14 +348,11 @@ int mysql_load(THD *thd,sql_exchange *ex
         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);
       }
 
     }


Attachment: [text/bzr-bundle]
Thread
bzr commit into mysql-5.1-bugteam branch (kristofer.pettersson:3365)Bug#50373Kristofer Pettersson3 May