From 45fe5879f4e3130acbe112a200dd79db03707d16 Mon Sep 17 00:00:00 2001 From: "msvensson@pilot.mysql.com" <> Date: Wed, 14 Feb 2007 14:44:34 +0100 Subject: [PATCH] Bug#18628 mysql-test-run: security problem(part1) - Implement --secure-file-priv= option that limits "load_file", "LOAD DATA" and "SELECT .. INTO OUTFILE" to work with files in specified dir. - Use above option for mysqld in mysql-test-run.pl --- mysql-test/mysql-test-run.pl | 6 ++++++ mysql-test/r/loaddata.result | 16 ++++++++++++++++ mysql-test/r/outfile.result | Bin 1159 -> 1382 bytes mysql-test/r/query_cache.result | 2 +- mysql-test/r/type_blob.result | 24 ++++++++++++------------ mysql-test/t/loaddata.test | 23 +++++++++++++++++++++++ mysql-test/t/outfile.test | 12 ++++++++++++ mysql-test/t/query_cache.test | 4 ++-- mysql-test/t/type_blob.test | 27 +++++++++++++-------------- sql/item_strfunc.cc | 5 +++++ sql/mysql_priv.h | 1 + sql/mysqld.cc | 20 +++++++++++++++++++- sql/set_var.cc | 4 ++++ sql/share/errmsg.txt | 2 +- sql/sql_class.cc | 12 ++++++++++-- sql/sql_class.h | 2 +- sql/sql_load.cc | 9 +++++++++ 17 files changed, 135 insertions(+), 34 deletions(-) diff --git a/mysql-test/mysql-test-run.pl b/mysql-test/mysql-test-run.pl index ca967655e06..c4ce4317fe7 100755 --- a/mysql-test/mysql-test-run.pl +++ b/mysql-test/mysql-test-run.pl @@ -3581,6 +3581,12 @@ sub mysqld_arguments ($$$$$) { mtr_add_arg($args, "%s--basedir=%s", $prefix, $path_my_basedir); mtr_add_arg($args, "%s--character-sets-dir=%s", $prefix, $path_charsetsdir); + if ( $mysql_version_id >= 50036) + { + # Prevent the started mysqld to access files outside of vardir + mtr_add_arg($args, "%s--secure-file-priv=%s", $prefix, $opt_vardir); + } + if ( $mysql_version_id >= 50000 ) { mtr_add_arg($args, "%s--log-bin-trust-function-creators", $prefix); diff --git a/mysql-test/r/loaddata.result b/mysql-test/r/loaddata.result index d415bd468e0..83c7b37d914 100644 --- a/mysql-test/r/loaddata.result +++ b/mysql-test/r/loaddata.result @@ -139,4 +139,20 @@ 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; +load data infile 'MYSQL_TEST_DIR/Makefile' into table t1; +ERROR HY000: The MySQL server is running with the --secure-file-priv option so it cannot execute this statement +select * from t1; +a b c +select load_file("MYSQL_TEST_DIR/Makefile"); +load_file("MYSQL_TEST_DIR/Makefile") +NULL drop table t1, t2; diff --git a/mysql-test/r/outfile.result b/mysql-test/r/outfile.result index 040dff576f87ba9c615679088f44846babb424f9..023c4ea205f4c5a038bf841e341968343f71ad5e 100644 GIT binary patch delta 232 zcmX|+v2MaZ5Ja0a+*gdK5~LG$i`yW91VR`$t{lf%;+5dex?KaQQu=rJD!zlW6s1{d zR&Qr#hRMU+hnL@82uh-vX#&+6Mg4h-GkvYWPTw*kbfQKv8>NTu^~e#JZp*`dQx(fY zQN6G8`QP4-Ug;Fa;Lp({Gs`^Rc_ptr(), mysql_real_data_home, "", 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))) + goto err; + if (!my_stat(path, &stat_info, MYF(0))) goto err; diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h index 8ffa73d1707..f27dabab832 100644 --- a/sql/mysql_priv.h +++ b/sql/mysql_priv.h @@ -1259,6 +1259,7 @@ extern my_bool opt_slave_compressed_protocol, use_temp_pool; extern my_bool opt_readonly, lower_case_file_system; extern my_bool opt_enable_named_pipe, opt_sync_frm, opt_allow_suspicious_udfs; extern my_bool opt_secure_auth; +extern char* opt_secure_file_priv; extern my_bool opt_log_slow_admin_statements; extern my_bool sp_automatic_privileges, opt_noacl; extern my_bool opt_old_style_user_limits, trust_function_creators; diff --git a/sql/mysqld.cc b/sql/mysqld.cc index e57e46bc30e..87760c3ded5 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -369,6 +369,7 @@ ulong opt_ndb_nodeid; my_bool opt_readonly, use_temp_pool, relay_log_purge; my_bool opt_sync_frm, opt_allow_suspicious_udfs; my_bool opt_secure_auth= 0; +char* opt_secure_file_priv= 0; my_bool opt_log_slow_admin_statements= 0; my_bool lower_case_file_system= 0; my_bool opt_large_pages= 0; @@ -1145,6 +1146,7 @@ void clean_up(bool print_message) #endif x_free(opt_bin_logname); x_free(opt_relay_logname); + x_free(opt_secure_file_priv); bitmap_free(&temp_pool); free_max_user_conn(); #ifdef HAVE_REPLICATION @@ -4698,7 +4700,8 @@ enum options_mysqld OPT_TABLE_LOCK_WAIT_TIMEOUT, OPT_PORT_OPEN_TIMEOUT, OPT_MERGE, - OPT_INNODB_ROLLBACK_ON_TIMEOUT + OPT_INNODB_ROLLBACK_ON_TIMEOUT, + OPT_SECURE_FILE_PRIV }; @@ -5337,6 +5340,10 @@ Can't be set to 1 if --log-slave-updates is used.", {"secure-auth", OPT_SECURE_AUTH, "Disallow authentication for accounts that have old (pre-4.1) passwords.", (gptr*) &opt_secure_auth, (gptr*) &opt_secure_auth, 0, GET_BOOL, NO_ARG, my_bool(0), 0, 0, 0, 0, 0}, + {"secure-file-priv", OPT_SECURE_FILE_PRIV, + "Limit LOAD DATA, SELECT ... OUTFILE, and LOAD_FILE() to files within specified directory", + (gptr*) &opt_secure_file_priv, (gptr*) &opt_secure_file_priv, 0, + GET_STR_ALLOC, REQUIRED_ARG, 0, 0, 0, 0, 0, 0}, {"server-id", OPT_SERVER_ID, "Uniquely identifies the server instance in the community of replication partners.", (gptr*) &server_id, (gptr*) &server_id, 0, GET_ULONG, REQUIRED_ARG, 0, 0, 0, @@ -6366,6 +6373,7 @@ static void mysql_init_variables(void) opt_logname= opt_update_logname= opt_binlog_index_name= opt_slow_logname= 0; opt_tc_log_file= (char *)"tc.log"; // no hostname in tc_log file name ! opt_secure_auth= 0; + opt_secure_file_priv= 0; opt_bootstrap= opt_myisam_log= 0; mqh_used= 0; segfaulted= kill_in_progress= 0; @@ -7404,6 +7412,16 @@ static void fix_paths(void) exit(1); } #endif /* HAVE_REPLICATION */ + /* + Convert the secure-file-priv option to system format, allowing + a quick strcmp to check if read or write is in an allowed dir + */ + if (opt_secure_file_priv) + { + convert_dirname(buff, opt_secure_file_priv, NullS); + my_free(opt_secure_file_priv, MYF(0)); + opt_secure_file_priv= my_strdup(buff, MYF(MY_FAE)); + } } diff --git a/sql/set_var.cc b/sql/set_var.cc index 9b02a192fe5..8562d052411 100644 --- a/sql/set_var.cc +++ b/sql/set_var.cc @@ -358,6 +358,8 @@ sys_query_cache_wlock_invalidate("query_cache_wlock_invalidate", &SV::query_cache_wlock_invalidate); #endif /* HAVE_QUERY_CACHE */ sys_var_bool_ptr sys_secure_auth("secure_auth", &opt_secure_auth); +sys_var_const_str_ptr sys_secure_file_priv("secure_file_priv", + &opt_secure_file_priv); sys_var_long_ptr sys_server_id("server_id", &server_id, fix_server_id); sys_var_bool_ptr sys_slave_compressed_protocol("slave_compressed_protocol", &opt_slave_compressed_protocol); @@ -719,6 +721,7 @@ sys_var *sys_variables[]= &sys_rpl_recovery_rank, &sys_safe_updates, &sys_secure_auth, + &sys_secure_file_priv, &sys_select_limit, &sys_server_id, #ifdef HAVE_REPLICATION @@ -1027,6 +1030,7 @@ struct show_var_st init_vars[]= { #endif {sys_rpl_recovery_rank.name,(char*) &sys_rpl_recovery_rank, SHOW_SYS}, {"secure_auth", (char*) &sys_secure_auth, SHOW_SYS}, + {"secure_file_priv", (char*) &sys_secure_file_priv, SHOW_SYS}, #ifdef HAVE_SMEM {"shared_memory", (char*) &opt_enable_shared_memory, SHOW_MY_BOOL}, {"shared_memory_base_name", (char*) &shared_memory_base_name, SHOW_CHAR_PTR}, diff --git a/sql/share/errmsg.txt b/sql/share/errmsg.txt index e09629a1f3e..adafdf2818b 100644 --- a/sql/share/errmsg.txt +++ b/sql/share/errmsg.txt @@ -5045,7 +5045,7 @@ ER_OPTION_PREVENTS_STATEMENT ger "Der MySQL-Server läuft mit der Option %s und kann diese Anweisung deswegen nicht ausführen" por "O servidor MySQL está rodando com a opção %s razão pela qual não pode executar esse commando" spa "El servidor MySQL está rodando con la opción %s tal que no puede ejecutar este comando" - swe "MySQL är startad med --skip-grant-tables. Pga av detta kan du inte använda detta kommando" + swe "MySQL är startad med %s. Pga av detta kan du inte använda detta kommando" ER_DUPLICATED_VALUE_IN_TYPE eng "Column '%-.100s' has duplicated value '%-.64s' in %s" ger "Feld '%-.100s' hat doppelten Wert '%-.64s' in %s" diff --git a/sql/sql_class.cc b/sql/sql_class.cc index 0794d4c797a..8fc0fb0a1b7 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -1084,7 +1084,7 @@ static File create_file(THD *thd, char *path, sql_exchange *exchange, IO_CACHE *cache) { File file; - uint option= MY_UNPACK_FILENAME; + uint option= MY_UNPACK_FILENAME | MY_RELATIVE_PATH; #ifdef DONT_ALLOW_FULL_LOAD_DATA_PATHS option|= MY_REPLACE_DIR; // Force use of db directory @@ -1097,7 +1097,15 @@ 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))) + { + /* Write only allowed to dir or subdir specified by secure_file_priv */ + my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--secure-file-priv"); + return -1; + } + if (!access(path, F_OK)) { my_error(ER_FILE_EXISTS_ERROR, MYF(0), exchange->file_name); diff --git a/sql/sql_class.h b/sql/sql_class.h index 05034ebd573..1fe5ef54043 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -1674,7 +1674,7 @@ public: #define SYSTEM_THREAD_SLAVE_SQL 4 /* - Used to hold information about file and file structure in exchainge + Used to hold information about file and file structure in exchange via non-DB file (...INTO OUTFILE..., ...LOAD DATA...) XXX: We never call destructor for objects of this class. */ diff --git a/sql/sql_load.cc b/sql/sql_load.cc index 0e4057d9ae4..ede70adc378 100644 --- a/sql/sql_load.cc +++ b/sql/sql_load.cc @@ -302,6 +302,15 @@ bool mysql_load(THD *thd,sql_exchange *ex,TABLE_LIST *table_list, if ((stat_info.st_mode & S_IFIFO) == S_IFIFO) is_fifo = 1; #endif + + if (opt_secure_file_priv && + 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); + } + } if ((file=my_open(name,O_RDONLY,MYF(MY_WME))) < 0) DBUG_RETURN(TRUE);