From: Date: July 26 2008 11:44am Subject: bzr commit into mysql-4.1 branch (holyfoot:2705) Bug#32167 List-Archive: http://lists.mysql.com/commits/50551 X-Bug: 32167 Message-Id: <20080726094412.B8FA92C380A5@hfmain.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit #At file:///home/hf/work/mysql_common/32167/ 2705 Alexey Botchkov 2008-07-26 Bug#32167 another privilege bypass with DATA/INDEX DIRECTORY. test_if_data_home_dir fixed to look into real path. Checks added to mi_open 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. mi_open_datafile now accepts 'original' file path to check if it's an allowed symlink. myisam/mi_static.c Bug#32167 another privilege bypass with DATA/INDEX DIRECTORY. myisam_test_invlaid_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.test Bug#32167 another privilege bypass with DATA/INDEX DIRECTORY. error codes corrected as some patch now rejected pointing inside datahome 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() implementsd my_realpath() now returns the 'realpath' even if a file isn't a symlink sql/mysql_priv.h Bug#32167 another privilege bypass with DATA/INDEX DIRECTORY. test_if_data_home_dir interface sql/mysqld.cc Bug#32167 another privilege bypass with DATA/INDEX DIRECTORY. myisam_test_invalid_symlik set with the 'test_if_data_home_dir' sql/sql_parse.cc Bug#32167 another privilege bypass with DATA/INDEX DIRECTORY. error messages corrected test_if_data_home_dir code fixed 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 mysql-test/t/symlink.test mysys/my_symlink.c sql/mysql_priv.h sql/mysqld.cc sql/sql_parse.cc === 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-26 09:39:31 +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-26 09:39:31 +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-26 09:39:31 +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,name,-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,name,-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,name,-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-26 09:39:31 +0000 @@ -75,7 +75,7 @@ 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], @@ -95,7 +95,15 @@ 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(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 +457,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, name, -1)) goto err; errpos=5; @@ -519,7 +527,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, name, old_info->dfile)) goto err; errpos=5; have_rtree= old_info->rtree_recursion_state != NULL; @@ -1153,12 +1161,30 @@ 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 *org_name, + File file_to_dup __attribute__((unused))) { + char *data_name= share->data_file_name; + char real_data_name[FN_REFLEN]; + + if (org_name) + { + fn_format(real_data_name,org_name,"",MI_NAME_DEXT,4); + if (my_is_symlink(real_data_name)) + { + if (my_realpath(real_data_name, real_data_name, MYF(0)) || + (*myisam_test_invalid_symlink)(real_data_name)) + { + my_errno= HA_WRONG_CREATE_OPTION; + return 1; + } + data_name= real_data_name; + } + } #ifdef USE_RAID if (share->base.raid_type) { - info->dfile=my_raid_open(share->data_file_name, + info->dfile=my_raid_open(data_name, share->mode | O_SHARE, share->base.raid_type, share->base.raid_chunks, @@ -1167,8 +1193,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(data_name, 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-26 09:39:31 +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-26 09:39:31 +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-26 09:39:31 +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 *orn_name, + 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-26 09:39:31 +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 'mysql-test/t/symlink.test' --- a/mysql-test/t/symlink.test 2008-02-29 12:56:41 +0000 +++ b/mysql-test/t/symlink.test 2008-07-26 09:39:31 +0000 @@ -67,7 +67,7 @@ drop table t1; # disable_query_log; ---error 1103,1103 +--error 1210, 1210 create table t1 (a int not null auto_increment, b char(16) not null, primary key (a)) engine=myisam data directory="tmp"; # Check that we cannot link over a table from another database. @@ -77,7 +77,7 @@ create database mysqltest; --error 1,1 create table mysqltest.t9 (a int not null auto_increment, b char(16) not null, primary key (a)) engine=myisam index directory="/this-dir-does-not-exist"; ---error 1103,1103 +--error 1210, 1210 create table mysqltest.t9 (a int not null auto_increment, b char(16) not null, primary key (a)) engine=myisam index directory="not-hard-path"; --error 1,1 === 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-26 09:39:31 +0000 @@ -107,38 +107,38 @@ int my_symlink(const char *content, cons #define BUFF_LEN FN_LEN #endif +int my_is_symlink(const char *filename __attribute__((unused))) +{ + struct stat stat_buff; + return !lstat(filename, &stat_buff) && S_ISLNK(stat_buff.st_mode); +} + + 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; + char *ptr; DBUG_ENTER("my_realpath"); - if (!(MyFlags & MY_RESOLVE_LINK) || - (!lstat(filename,&stat_buff) && S_ISLNK(stat_buff.st_mode))) + DBUG_PRINT("info",("executing realpath")); + if ((ptr=realpath(filename,buff))) + strmake(to,ptr,FN_REFLEN-1); + else { - char *ptr; - DBUG_PRINT("info",("executing realpath")); - if ((ptr=realpath(filename,buff))) - { - strmake(to,ptr,FN_REFLEN-1); - } - else - { - /* - Realpath didn't work; Use my_load_path() which is a poor substitute - original name but will at least be able to resolve paths that starts - with '.'. - */ - DBUG_PRINT("error",("realpath failed with errno: %d", errno)); - my_errno=errno; - if (MyFlags & MY_WME) - my_error(EE_REALPATH, MYF(0), filename, my_errno); - my_load_path(to, filename, NullS); - result= -1; - } + /* + Realpath didn't work; Use my_load_path() which is a poor substitute + original name but will at least be able to resolve paths that starts + with '.'. + */ + DBUG_PRINT("error",("realpath failed with errno: %d", errno)); + my_errno=errno; + if (MyFlags & MY_WME) + my_error(EE_REALPATH, MYF(0), filename, my_errno); + my_load_path(to, filename, NullS); + result= -1; } DBUG_RETURN(result); #else @@ -146,3 +146,4 @@ int my_realpath(char *to, const char *fi return 0; #endif } + === 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-26 09:39:31 +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-26 09:39:31 +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-26 09:39:31 +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 +