List:Commits« Previous MessageNext Message »
From:Alexey Botchkov Date:July 18 2008 12:03pm
Subject:bzr commit into mysql-4.1 branch (holyfoot:2705) Bug#32167
View as plain text  
#At file:///home/hf/work/mysql_common/32167/

 2705 Alexey Botchkov	2008-07-18
      bug#32167 another privilege bypass with DATA/INDEX DIRECTORY.
      
      test_if_data_home_dir fixed to look into the real path.
      Checks added to mi_optn for symlinks into data home directory.
modified:
  include/my_sys.h
  include/myisam.h
  myisam/mi_check.c
  myisam/mi_open.c
  myisam/mi_static.c
  myisam/myisamchk.c
  myisam/myisamdef.h
  mysql-test/r/symlink.result
  mysys/my_symlink.c
  sql/mysql_priv.h
  sql/mysqld.cc
  sql/sql_parse.cc

per-file messages:
  include/my_sys.h
    bug#32167 another privilege bypass with DATA/INDEX DIRECTORY.
    
    my_is_symlink interface added.
  include/myisam.h
    bug#32167 another privilege bypass with DATA/INDEX DIRECTORY.
    
    myisam_test_invalid_symlink interface added.
  myisam/mi_check.c
    bug#32167 another privilege bypass with DATA/INDEX DIRECTORY.
    
    mi_open_datafile calls modified.
  myisam/mi_open.c
    bug#32167 another privilege bypass with DATA/INDEX DIRECTORY.
    
    code added to mi_open to check for symlinks into data home directory.
  myisam/mi_static.c
    bug#32167 another privilege bypass with DATA/INDEX DIRECTORY.
    
    myisam_test_invalid_symlink defined.
  myisam/myisamchk.c
    bug#32167 another privilege bypass with DATA/INDEX DIRECTORY.
    
    mi_open_datafile call modified.
  myisam/myisamdef.h
    bug#32167 another privilege bypass with DATA/INDEX DIRECTORY.
    
    mi_open_datafile interface modified - real_path parameter added.
  mysql-test/r/symlink.result
    bug#32167 another privilege bypass with DATA/INDEX DIRECTORY.
    
    error messages corrected in the result.
  mysys/my_symlink.c
    bug#32167 another privilege bypass with DATA/INDEX DIRECTORY.
    
    my_is_symlink() implemented.
  sql/mysql_priv.h
    bug#32167 another privilege bypass with DATA/INDEX DIRECTORY.
    
    test_if_data_home_dir interface added.
  sql/mysqld.cc
    bug#32167 another privilege bypass with DATA/INDEX DIRECTORY.
    
    myisam_test_invalid_symlink set with the 'test_if_data_home_dir'
    mysql_unpacked_real_data_home initialization errors corrected.
  sql/sql_parse.cc
    bug#32167 another privilege bypass with DATA/INDEX DIRECTORY.
    
    error message corrected.
=== modified file 'include/my_sys.h'
--- a/include/my_sys.h	2007-10-24 11:09:30 +0000
+++ b/include/my_sys.h	2008-07-18 12:02:39 +0000
@@ -539,6 +539,7 @@ extern int my_close(File Filedes,myf MyF
 extern File my_dup(File file, myf MyFlags);
 extern int my_mkdir(const char *dir, int Flags, myf MyFlags);
 extern int my_readlink(char *to, const char *filename, myf MyFlags);
+extern int my_is_symlink(const char *filename);
 extern int my_realpath(char *to, const char *filename, myf MyFlags);
 extern File my_create_with_symlink(const char *linkname, const char *filename,
 				   int createflags, int access_flags,

=== modified file 'include/myisam.h'
--- a/include/myisam.h	2006-10-09 17:26:55 +0000
+++ b/include/myisam.h	2008-07-18 12:02:39 +0000
@@ -195,6 +195,10 @@ extern my_bool myisam_concurrent_insert;
 extern my_off_t myisam_max_temp_length,myisam_max_extra_temp_length;
 extern ulong myisam_bulk_insert_tree_size, myisam_data_pointer_size;
 
+/* usually used to check if a symlink points into the mysql data home */
+/* which is normally forbidden                                        */
+extern int (*myisam_test_invalid_symlink)(const char *filename);
+
 	/* Prototypes for myisam-functions */
 
 extern int mi_close(struct st_myisam_info *file);

=== modified file 'myisam/mi_check.c'
--- a/myisam/mi_check.c	2007-12-13 10:38:01 +0000
+++ b/myisam/mi_check.c	2008-07-18 12:02:39 +0000
@@ -1610,7 +1610,7 @@ err:
 			    DATA_TMP_EXT, share->base.raid_chunks,
 			    (param->testflag & T_BACKUP_DATA ?
 			     MYF(MY_REDEL_MAKE_BACKUP): MYF(0))) ||
-	  mi_open_datafile(info,share,-1))
+	  mi_open_datafile(info,share,NULL,-1))
 	got_error=1;
     }
   }
@@ -2390,7 +2390,7 @@ err:
 			    DATA_TMP_EXT, share->base.raid_chunks,
 			    (param->testflag & T_BACKUP_DATA ?
 			     MYF(MY_REDEL_MAKE_BACKUP): MYF(0))) ||
-	  mi_open_datafile(info,share,-1))
+	  mi_open_datafile(info,share,NULL,-1))
 	got_error=1;
     }
   }
@@ -2927,7 +2927,7 @@ err:
 			    DATA_TMP_EXT, share->base.raid_chunks,
 			    (param->testflag & T_BACKUP_DATA ?
 			     MYF(MY_REDEL_MAKE_BACKUP): MYF(0))) ||
-	  mi_open_datafile(info,share,-1))
+	  mi_open_datafile(info,share,NULL,-1))
 	got_error=1;
     }
   }

=== modified file 'myisam/mi_open.c'
--- a/myisam/mi_open.c	2007-11-07 08:55:28 +0000
+++ b/myisam/mi_open.c	2008-07-18 12:02:39 +0000
@@ -75,11 +75,11 @@ MI_INFO *test_if_reopen(char *filename)
 
 MI_INFO *mi_open(const char *name, int mode, uint open_flags)
 {
-  int lock_error,kfile,open_mode,save_errno,have_rtree=0;
+  int lock_error,kfile,open_mode,save_errno,have_rtree=0, realpath_err;
   uint i,j,len,errpos,head_length,base_pos,offset,info_length,keys,
     key_parts,unique_key_parts,fulltext_keys,uniques;
   char name_buff[FN_REFLEN], org_name[FN_REFLEN], index_name[FN_REFLEN],
-       data_name[FN_REFLEN];
+       data_name[FN_REFLEN], real_data_name[FN_REFLEN];
   char *disk_cache, *disk_pos, *end_pos;
   MI_INFO info,*m_info,*old_info;
   MYISAM_SHARE share_buff,*share;
@@ -95,7 +95,24 @@ MI_INFO *mi_open(const char *name, int m
   head_length=sizeof(share_buff.state.header);
   bzero((byte*) &info,sizeof(info));
 
-  my_realpath(name_buff, fn_format(org_name,name,"",MI_NAME_IEXT,4),MYF(0));
+  realpath_err= my_realpath(real_data_name,
+                            fn_format(org_name,name,"",MI_NAME_DEXT,4), MYF(0));
+  if (my_is_symlink(org_name) &&
+       (realpath_err || (*myisam_test_invalid_symlink)(real_data_name)))
+  {
+    my_errno= HA_WRONG_CREATE_OPTION;
+    DBUG_RETURN (NULL);
+  }
+
+  realpath_err= my_realpath(name_buff,
+                  fn_format(org_name,name,"",MI_NAME_IEXT,4),MYF(0));
+  if (my_is_symlink(org_name) &&
+      (realpath_err || (*myisam_test_invalid_symlink)(name_buff)))
+  {
+    my_errno= HA_WRONG_CREATE_OPTION;
+    DBUG_RETURN (NULL);
+  }
+
   pthread_mutex_lock(&THR_LOCK_myisam);
   if (!(old_info=test_if_reopen(name_buff)))
   {
@@ -449,7 +466,7 @@ MI_INFO *mi_open(const char *name, int m
       lock_error=1;			/* Database unlocked */
     }
 
-    if (mi_open_datafile(&info, share, -1))
+    if (mi_open_datafile(&info, share, real_data_name, -1))
       goto err;
     errpos=5;
 
@@ -519,7 +536,7 @@ MI_INFO *mi_open(const char *name, int m
       my_errno=EACCES;				/* Can't open in write mode */
       goto err;
     }
-    if (mi_open_datafile(&info, share, old_info->dfile))
+    if (mi_open_datafile(&info, share, real_data_name, old_info->dfile))
       goto err;
     errpos=5;
     have_rtree= old_info->rtree_recursion_state != NULL;
@@ -1153,12 +1170,15 @@ The argument file_to_dup is here for the
 exist a dup()-like call that would give us two different file descriptors.
 *************************************************************************/
 
-int mi_open_datafile(MI_INFO *info, MYISAM_SHARE *share, File file_to_dup __attribute__((unused)))
+int mi_open_datafile(MI_INFO *info, MYISAM_SHARE *share, const char *real_path,
+                     File file_to_dup __attribute__((unused)))
 {
+  if (!real_path)
+    real_path= share->data_file_name;
 #ifdef USE_RAID
   if (share->base.raid_type)
   {
-    info->dfile=my_raid_open(share->data_file_name,
+    info->dfile=my_raid_open(real_path,
 			     share->mode | O_SHARE,
 			     share->base.raid_type,
 			     share->base.raid_chunks,
@@ -1167,8 +1187,7 @@ int mi_open_datafile(MI_INFO *info, MYIS
   }
   else
 #endif
-    info->dfile=my_open(share->data_file_name, share->mode | O_SHARE,
-			MYF(MY_WME));
+    info->dfile=my_open(real_path, share->mode | O_SHARE, MYF(MY_WME));
   return info->dfile >= 0 ? 0 : 1;
 }
 

=== modified file 'myisam/mi_static.c'
--- a/myisam/mi_static.c	2005-09-05 11:31:42 +0000
+++ b/myisam/mi_static.c	2008-07-18 12:02:39 +0000
@@ -43,6 +43,15 @@ my_off_t myisam_max_temp_length= MAX_FIL
 ulong    myisam_bulk_insert_tree_size=8192*1024;
 ulong    myisam_data_pointer_size=4;
 
+
+static int always_valid(const char *filename)
+{
+  return 0;
+}
+
+int (*myisam_test_invalid_symlink)(const char *filename)= always_valid;
+
+
 /*
   read_vec[] is used for converting between P_READ_KEY.. and SEARCH_
   Position is , == , >= , <= , > , <

=== modified file 'myisam/myisamchk.c'
--- a/myisam/myisamchk.c	2007-11-07 08:55:28 +0000
+++ b/myisam/myisamchk.c	2008-07-18 12:02:39 +0000
@@ -1039,7 +1039,7 @@ static int myisamchk(MI_CHECK *param, my
 	  error|=change_to_newfile(filename,MI_NAME_DEXT,DATA_TMP_EXT,
 				   raid_chunks,
 				   MYF(0));
-	  if (mi_open_datafile(info,info->s, -1))
+	  if (mi_open_datafile(info,info->s, NULL, -1))
 	    error=1;
 	  param->out_flag&= ~O_NEW_DATA; /* We are using new datafile */
 	  param->read_cache.file=info->dfile;

=== modified file 'myisam/myisamdef.h'
--- a/myisam/myisamdef.h	2007-04-10 20:40:35 +0000
+++ b/myisam/myisamdef.h	2008-07-18 12:02:39 +0000
@@ -731,7 +731,8 @@ void mi_disable_non_unique_index(MI_INFO
 
 extern MI_INFO *test_if_reopen(char *filename);
 my_bool check_table_is_closed(const char *name, const char *where);
-int mi_open_datafile(MI_INFO *info, MYISAM_SHARE *share, File file_to_dup);
+int mi_open_datafile(MI_INFO *info, MYISAM_SHARE *share, const char *real_path,
+                     File file_to_dup);
 int mi_open_keyfile(MYISAM_SHARE *share);
 void mi_setup_functions(register MYISAM_SHARE *share);
 

=== modified file 'mysql-test/r/symlink.result'
--- a/mysql-test/r/symlink.result	2008-02-29 12:56:41 +0000
+++ b/mysql-test/r/symlink.result	2008-07-18 12:02:39 +0000
@@ -138,13 +138,13 @@ drop table t1;
 deallocate prepare stmt;
 CREATE TABLE t1(a INT)
 DATA DIRECTORY='TEST_DIR/var/master-data/test';
-ERROR HY000: Incorrect arguments to DATA DIRECORY
+ERROR HY000: Incorrect arguments to DATA DIRECTORY
 CREATE TABLE t1(a INT)
 DATA DIRECTORY='TEST_DIR/var/master-data/';
-ERROR HY000: Incorrect arguments to DATA DIRECORY
+ERROR HY000: Incorrect arguments to DATA DIRECTORY
 CREATE TABLE t1(a INT)
 INDEX DIRECTORY='TEST_DIR/var/master-data';
-ERROR HY000: Incorrect arguments to INDEX DIRECORY
+ERROR HY000: Incorrect arguments to INDEX DIRECTORY
 CREATE TABLE t1(a INT)
 INDEX DIRECTORY='TEST_DIR/var/master-data_var';
 ERROR HY000: Can't create/write to file 'TEST_DIR/var/master-data_var/t1.MYI' (Errcode: 2)

=== modified file 'mysys/my_symlink.c'
--- a/mysys/my_symlink.c	2005-01-18 01:49:39 +0000
+++ b/mysys/my_symlink.c	2008-07-18 12:02:39 +0000
@@ -107,17 +107,26 @@ int my_symlink(const char *content, cons
 #define BUFF_LEN FN_LEN
 #endif
 
+int my_is_symlink(const char *filename __attribute__((unused)))
+{
+#if defined(HAVE_REALPATH) && !defined(HAVE_purify) && !defined(HAVE_BROKEN_REALPATH)
+  struct stat stat_buff;
+  return !lstat(filename, &stat_buff) && S_ISLNK(stat_buff.st_mode);
+#else
+  return 0;
+#endif /*HAVE_REALPATH*/
+}
+
+
 int my_realpath(char *to, const char *filename,
 		myf MyFlags __attribute__((unused)))
 {
 #if defined(HAVE_REALPATH) && !defined(HAVE_purify) && !defined(HAVE_BROKEN_REALPATH)
   int result=0;
   char buff[BUFF_LEN];
-  struct stat stat_buff;
   DBUG_ENTER("my_realpath");
 
-  if (!(MyFlags & MY_RESOLVE_LINK) ||
-      (!lstat(filename,&stat_buff) && S_ISLNK(stat_buff.st_mode)))
+  if (!(MyFlags & MY_RESOLVE_LINK) || my_is_symlink(filename))
   {
     char *ptr;
     DBUG_PRINT("info",("executing realpath"));

=== modified file 'sql/mysql_priv.h'
--- a/sql/mysql_priv.h	2008-02-29 09:55:00 +0000
+++ b/sql/mysql_priv.h	2008-07-18 12:02:39 +0000
@@ -892,6 +892,7 @@ extern char *mysql_data_home,server_vers
 	    mysql_real_data_home[], *opt_mysql_tmpdir, mysql_charsets_dir[],
 	    mysql_unpacked_real_data_home[],
             def_ft_boolean_syntax[sizeof(ft_boolean_syntax)];
+extern int mysql_unpacked_real_data_home_len;
 #define mysql_tmpdir (my_tmpdir(&mysql_tmpdir_list))
 extern MY_TMPDIR mysql_tmpdir_list;
 extern const char *command_name[];
@@ -1332,3 +1333,6 @@ bool check_stack_overrun(THD *thd,char *
 inline void kill_delayed_threads(void) {}
 #define check_stack_overrun(A, B) 0
 #endif
+
+
+extern "C" int test_if_data_home_dir(const char *dir);

=== modified file 'sql/mysqld.cc'
--- a/sql/mysqld.cc	2008-02-29 09:55:00 +0000
+++ b/sql/mysqld.cc	2008-07-18 12:02:39 +0000
@@ -391,6 +391,7 @@ char compiled_default_collation_name[]= 
 char *language_ptr, *default_collation_name, *default_character_set_name;
 char mysql_data_home_buff[2], *mysql_data_home=mysql_real_data_home;
 char mysql_unpacked_real_data_home[FN_REFLEN];
+int mysql_unpacked_real_data_home_len;
 struct passwd *user_info;
 char server_version[SERVER_VERSION_LENGTH];
 char *mysqld_unix_port, *opt_mysql_tmpdir;
@@ -5905,6 +5906,7 @@ static void mysql_init_variables(void)
   /* Things reset to zero */
   opt_skip_slave_start= opt_reckless_slave = 0;
   mysql_home[0]= pidfile_name[0]= log_error_file[0]= 0;
+  myisam_test_invalid_symlink= test_if_data_home_dir;
   opt_log= opt_update_log= opt_bin_log= opt_slow_log= 0;
   opt_disable_networking= opt_skip_show_db=0;
   opt_logname= opt_update_logname= opt_binlog_index_name= opt_slow_logname=0;
@@ -6897,9 +6899,11 @@ static void fix_paths(void)
     pos[1]= 0;
   }
   convert_dirname(mysql_real_data_home,mysql_real_data_home,NullS);
-  (void) fn_format(buff, mysql_real_data_home, "", "",
-                   (MY_RETURN_REAL_PATH|MY_RESOLVE_SYMLINKS));
-  (void) unpack_dirname(mysql_unpacked_real_data_home, buff);
+  my_realpath(mysql_unpacked_real_data_home, mysql_real_data_home, MYF(0));
+  mysql_unpacked_real_data_home_len= strlen(mysql_unpacked_real_data_home);
+  if (mysql_unpacked_real_data_home[mysql_unpacked_real_data_home_len-1] == FN_LIBCHAR)
+    --mysql_unpacked_real_data_home_len;
+
   convert_dirname(language,language,NullS);
   (void) my_load_path(mysql_home,mysql_home,""); // Resolve current dir
   (void) my_load_path(mysql_real_data_home,mysql_real_data_home,mysql_home);

=== modified file 'sql/sql_parse.cc'
--- a/sql/sql_parse.cc	2008-02-29 09:55:00 +0000
+++ b/sql/sql_parse.cc	2008-07-18 12:02:39 +0000
@@ -66,7 +66,6 @@ static bool append_file_to_dir(THD *thd,
              
 static TABLE_LIST* get_table_by_alias(TABLE_LIST* tl, const char* db,
   const char* alias);
-static bool test_if_data_home_dir(const char *dir);
 
 const char *any_db="*any*";	// Special symbol for check_access
 
@@ -2535,13 +2534,13 @@ mysql_execute_command(THD *thd)
 
     if (test_if_data_home_dir(lex->create_info.data_file_name))
     {
-      my_error(ER_WRONG_ARGUMENTS,MYF(0),"DATA DIRECORY");
+      my_error(ER_WRONG_ARGUMENTS,MYF(0),"DATA DIRECTORY");
       res= -1;
       break;
     }
     if (test_if_data_home_dir(lex->create_info.index_file_name))
     {
-      my_error(ER_WRONG_ARGUMENTS,MYF(0),"INDEX DIRECORY");
+      my_error(ER_WRONG_ARGUMENTS,MYF(0),"INDEX DIRECTORY");
       res= -1;
       break;
     }
@@ -5951,10 +5950,12 @@ Item *negate_expression(THD *thd, Item *
     1	error  
 */
 
-static bool test_if_data_home_dir(const char *dir)
+C_MODE_START
+
+int test_if_data_home_dir(const char *dir)
 {
-  char path[FN_REFLEN], conv_path[FN_REFLEN];
-  uint dir_len, home_dir_len= strlen(mysql_unpacked_real_data_home);
+  char path[FN_REFLEN];
+  uint dir_len;
   DBUG_ENTER("test_if_data_home_dir");
 
   if (!dir)
@@ -5962,20 +5963,28 @@ static bool test_if_data_home_dir(const 
 
   (void) fn_format(path, dir, "", "",
                    (MY_RETURN_REAL_PATH|MY_RESOLVE_SYMLINKS));
-  dir_len= unpack_dirname(conv_path, dir);
 
-  if (home_dir_len <= dir_len)
+  dir_len= strlen(path);
+  if (mysql_unpacked_real_data_home_len<= dir_len)
   {
+    if (dir_len > mysql_unpacked_real_data_home_len &&
+        path[mysql_unpacked_real_data_home_len] != FN_LIBCHAR)
+      DBUG_RETURN(0);
+
     if (lower_case_file_system)
     {
-      if (!my_strnncoll(default_charset_info, (const uchar*) conv_path,
-                        home_dir_len,
+      if (!my_strnncoll(default_charset_info, (const uchar*) path,
+                        mysql_unpacked_real_data_home_len,
                         (const uchar*) mysql_unpacked_real_data_home,
-                        home_dir_len))
+                        mysql_unpacked_real_data_home_len))
         DBUG_RETURN(1);
     }
-    else if (!memcmp(conv_path, mysql_unpacked_real_data_home, home_dir_len))
+    else if (!memcmp(path, mysql_unpacked_real_data_home,
+                     mysql_unpacked_real_data_home_len))
       DBUG_RETURN(1);
   }
   DBUG_RETURN(0);
 }
+
+C_MODE_END
+

Thread
bzr commit into mysql-4.1 branch (holyfoot:2705) Bug#32167Alexey Botchkov18 Jul
  • Re: bzr commit into mysql-4.1 branch (holyfoot:2705) Bug#32167Sergei Golubchik21 Jul
  • Re: bzr commit into mysql-4.1 branch (holyfoot:2705) Bug#32167Alexey Botchkov23 Jul
    • Re: bzr commit into mysql-4.1 branch (holyfoot:2705) Bug#32167Sergei Golubchik23 Jul
  • Re: bzr commit into mysql-4.1 branch (holyfoot:2705) Bug#32167Alexey Botchkov23 Jul
  • Re: bzr commit into mysql-4.1 branch (holyfoot:2705) Bug#32167Alexey Botchkov24 Jul
    • Re: bzr commit into mysql-4.1 branch (holyfoot:2705) Bug#32167Sergei Golubchik24 Jul