summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGeorgi Kodinov <Georgi.Kodinov@Oracle.com>2011-04-28 12:22:41 +0300
committerGeorgi Kodinov <Georgi.Kodinov@Oracle.com>2011-04-28 12:22:41 +0300
commit4c5dfc00f7a2ea3cf9475455e2749a96bfc38344 (patch)
tree65c1bdb17e8f4d53ba52f73aef750d72c771c28b
parent8f449c36defd0ad1e25f212026a4929899458336 (diff)
downloadmariadb-git-4c5dfc00f7a2ea3cf9475455e2749a96bfc38344.tar.gz
Bug #11764517: 57359: POSSIBLE TO CIRCUMVENT SECURE_FILE_PRIV
USING '..' ON WINDOWS Backport of the fix to 5.0 (to be null-merged to 5.1). Moved the test into the main test suite. Made mysql-test-run.pl to not use symlinks for sdtdata as the symlinks are now properly recognized by secure_file_priv. Made sure the paths in load_file(), LOAD DATA and SELECT .. INTO OUTFILE that are checked against secure_file_priv in a correct way similarly to 5.1 by the extended is_secure_file_path() backport before the comparison. Added an extensive test with all the variants of upper/lower case, slash/backslash and case sensitivity. Added few comments to the code.
-rwxr-xr-xmysql-test/mysql-test-run.pl14
-rw-r--r--mysql-test/r/secure_file_priv_win.result38
-rw-r--r--mysql-test/t/secure_file_priv_win-master.opt1
-rw-r--r--mysql-test/t/secure_file_priv_win.test79
-rw-r--r--mysys/my_symlink.c17
-rw-r--r--sql/item_strfunc.cc3
-rw-r--r--sql/mysql_priv.h2
-rw-r--r--sql/mysqld.cc58
-rw-r--r--sql/sql_class.cc3
-rw-r--r--sql/sql_load.cc48
10 files changed, 223 insertions, 40 deletions
diff --git a/mysql-test/mysql-test-run.pl b/mysql-test/mysql-test-run.pl
index f0100593516..f43cadd3784 100755
--- a/mysql-test/mysql-test-run.pl
+++ b/mysql-test/mysql-test-run.pl
@@ -2412,17 +2412,9 @@ sub setup_vardir() {
mkpath("$data_dir/test");
}
- # Make a link std_data_ln in var/ that points to std_data
- if ( ! $glob_win32 )
- {
- symlink("$glob_mysql_test_dir/std_data", "$opt_vardir/std_data_ln");
- }
- else
- {
- # on windows, copy all files from std_data into var/std_data_ln
- mkpath("$opt_vardir/std_data_ln");
- mtr_copy_dir("$glob_mysql_test_dir/std_data", "$opt_vardir/std_data_ln");
- }
+ # copy all files from std_data into var/std_data_ln
+ mkpath("$opt_vardir/std_data_ln");
+ mtr_copy_dir("$glob_mysql_test_dir/std_data", "$opt_vardir/std_data_ln");
# Remove old log files
foreach my $name (glob("r/*.progress r/*.log r/*.warnings"))
diff --git a/mysql-test/r/secure_file_priv_win.result b/mysql-test/r/secure_file_priv_win.result
new file mode 100644
index 00000000000..497a5d04b1f
--- /dev/null
+++ b/mysql-test/r/secure_file_priv_win.result
@@ -0,0 +1,38 @@
+CREATE TABLE t1 (c1 longtext);
+INSERT INTO t1 values ('a');
+SELECT * FROM t1 INTO OUTFILE 'd:/mysql/work/test-5.0-security/mysql-test/var/tmp/B11764517.tmp';
+show global variables like 'secure_file_priv';
+Variable_name Value
+secure_file_priv MYSQL_TMP_DIR/
+SELECT load_file('MYSQL_TMP_DIR\\B11764517.tmp') AS x;
+x
+a
+
+SELECT load_file('MYSQL_TMP_DIR/B11764517.tmp') AS x;
+x
+a
+
+SELECT load_file('MYSQL_TMP_DIR_UCASE/B11764517.tmp') AS x;
+x
+a
+
+SELECT load_file('MYSQL_TMP_DIR_LCASE/B11764517.tmp') AS x;
+x
+a
+
+SELECT load_file('MYSQL_TMP_DIR\\..a..\\..\\..\\B11764517.tmp') AS x;
+x
+NULL
+LOAD DATA INFILE 'MYSQL_TMP_DIR\\B11764517.tmp' INTO TABLE t1;
+LOAD DATA INFILE 'MYSQL_TMP_DIR/B11764517.tmp' INTO TABLE t1;
+LOAD DATA INFILE 'MYSQL_TMP_DIR_UCASE/B11764517.tmp' INTO TABLE t1;
+LOAD DATA INFILE 'MYSQL_TMP_DIR_LCASE/B11764517.tmp' INTO TABLE t1;
+LOAD DATA INFILE "MYSQL_TMP_DIR\\..a..\\..\\..\\B11764517.tmp" 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 INTO OUTFILE 'MYSQL_TMP_DIR\\..a..\\..\\..\\B11764517-2.tmp';
+ERROR HY000: The MySQL server is running with the --secure-file-priv option so it cannot execute this statement
+SELECT * FROM t1 INTO OUTFILE 'MYSQL_TMP_DIR\\B11764517-2.tmp';
+SELECT * FROM t1 INTO OUTFILE 'MYSQL_TMP_DIR/B11764517-3.tmp';
+SELECT * FROM t1 INTO OUTFILE 'MYSQL_TMP_DIR_UCASE/B11764517-4.tmp';
+SELECT * FROM t1 INTO OUTFILE 'MYSQL_TMP_DIR_LCASE/B11764517-5.tmp';
+DROP TABLE t1;
diff --git a/mysql-test/t/secure_file_priv_win-master.opt b/mysql-test/t/secure_file_priv_win-master.opt
new file mode 100644
index 00000000000..e9a43a5584d
--- /dev/null
+++ b/mysql-test/t/secure_file_priv_win-master.opt
@@ -0,0 +1 @@
+--secure_file_priv=$MYSQL_TMP_DIR
diff --git a/mysql-test/t/secure_file_priv_win.test b/mysql-test/t/secure_file_priv_win.test
new file mode 100644
index 00000000000..07e012e42b4
--- /dev/null
+++ b/mysql-test/t/secure_file_priv_win.test
@@ -0,0 +1,79 @@
+#
+# Bug58747 breaks secure_file_priv+not secure yet+still accesses other folders
+#
+
+# we do the windows specific relative directory testing
+
+--source include/windows.inc
+
+CREATE TABLE t1 (c1 longtext);
+INSERT INTO t1 values ('a');
+
+LET $MYSQL_TMP_DIR_UCASE= `SELECT upper('$MYSQL_TMP_DIR')`;
+LET $MYSQL_TMP_DIR_LCASE= `SELECT lower('$MYSQL_TMP_DIR')`;
+
+#create the file
+--replace_result $MYSQL_TMP_DIR MYSQL_TMP_DIR
+eval SELECT * FROM t1 INTO OUTFILE '$MYSQL_TMP_DIR_LCASE/B11764517.tmp';
+
+--replace_result $MYSQL_TMP_DIR MYSQL_TMP_DIR
+show global variables like 'secure_file_priv';
+
+--replace_result $MYSQL_TMP_DIR MYSQL_TMP_DIR
+eval SELECT load_file('$MYSQL_TMP_DIR\\\\B11764517.tmp') AS x;
+
+--replace_result $MYSQL_TMP_DIR MYSQL_TMP_DIR
+eval SELECT load_file('$MYSQL_TMP_DIR/B11764517.tmp') AS x;
+
+--replace_result $MYSQL_TMP_DIR_UCASE MYSQL_TMP_DIR_UCASE
+eval SELECT load_file('$MYSQL_TMP_DIR_UCASE/B11764517.tmp') AS x;
+
+--replace_result $MYSQL_TMP_DIR_LCASE MYSQL_TMP_DIR_LCASE
+eval SELECT load_file('$MYSQL_TMP_DIR_LCASE/B11764517.tmp') AS x;
+
+--replace_result $MYSQL_TMP_DIR MYSQL_TMP_DIR
+eval SELECT load_file('$MYSQL_TMP_DIR\\\\..a..\\\\..\\\\..\\\\B11764517.tmp') AS x;
+
+--replace_result $MYSQL_TMP_DIR MYSQL_TMP_DIR
+eval LOAD DATA INFILE '$MYSQL_TMP_DIR\\\\B11764517.tmp' INTO TABLE t1;
+
+--replace_result $MYSQL_TMP_DIR MYSQL_TMP_DIR
+eval LOAD DATA INFILE '$MYSQL_TMP_DIR/B11764517.tmp' INTO TABLE t1;
+
+--replace_result $MYSQL_TMP_DIR_UCASE MYSQL_TMP_DIR_UCASE
+eval LOAD DATA INFILE '$MYSQL_TMP_DIR_UCASE/B11764517.tmp' INTO TABLE t1;
+
+--replace_result $MYSQL_TMP_DIR_LCASE MYSQL_TMP_DIR_LCASE
+eval LOAD DATA INFILE '$MYSQL_TMP_DIR_LCASE/B11764517.tmp' INTO TABLE t1;
+
+--replace_result $MYSQL_TMP_DIR MYSQL_TMP_DIR
+--error ER_OPTION_PREVENTS_STATEMENT
+eval LOAD DATA INFILE "$MYSQL_TMP_DIR\\\\..a..\\\\..\\\\..\\\\B11764517.tmp" into table t1;
+
+--replace_result $MYSQL_TMP_DIR MYSQL_TMP_DIR
+--error ER_OPTION_PREVENTS_STATEMENT
+eval SELECT * FROM t1 INTO OUTFILE '$MYSQL_TMP_DIR\\\\..a..\\\\..\\\\..\\\\B11764517-2.tmp';
+
+--replace_result $MYSQL_TMP_DIR MYSQL_TMP_DIR
+eval SELECT * FROM t1 INTO OUTFILE '$MYSQL_TMP_DIR\\\\B11764517-2.tmp';
+
+--replace_result $MYSQL_TMP_DIR MYSQL_TMP_DIR
+eval SELECT * FROM t1 INTO OUTFILE '$MYSQL_TMP_DIR/B11764517-3.tmp';
+
+--replace_result $MYSQL_TMP_DIR_UCASE MYSQL_TMP_DIR_UCASE
+eval SELECT * FROM t1 INTO OUTFILE '$MYSQL_TMP_DIR_UCASE/B11764517-4.tmp';
+
+--replace_result $MYSQL_TMP_DIR_LCASE MYSQL_TMP_DIR_LCASE
+eval SELECT * FROM t1 INTO OUTFILE '$MYSQL_TMP_DIR_LCASE/B11764517-5.tmp';
+
+--error 0,1
+--remove_file $MYSQL_TMP_DIR/B11764517.tmp;
+--error 0,1
+--remove_file $MYSQL_TMP_DIR/B11764517-2.tmp;
+--error 0,1
+--remove_file $MYSQL_TMP_DIR/B11764517-3.tmp;
+--error 0,1
+--remove_file $MYSQL_TMP_DIR/B11764517-4.tmp;
+--error 0,1
+--remove_file $MYSQL_TMP_DIR/B11764517-5.tmp;
+DROP TABLE t1;
diff --git a/mysys/my_symlink.c b/mysys/my_symlink.c
index 7f2be5644e8..e17cd8bbe0c 100644
--- a/mysys/my_symlink.c
+++ b/mysys/my_symlink.c
@@ -149,8 +149,23 @@ int my_realpath(char *to, const char *filename,
result= -1;
}
DBUG_RETURN(result);
+#elif defined(_WIN32)
+ int ret= GetFullPathName(filename,FN_REFLEN, to, NULL);
+ if (ret == 0 || ret > FN_REFLEN)
+ {
+ my_errno= (ret > FN_REFLEN) ? ENAMETOOLONG : GetLastError();
+ if (MyFlags & MY_WME)
+ my_error(EE_REALPATH, MYF(0), filename, my_errno);
+ /*
+ GetFullPathName 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 '.'.
+ */
+ my_load_path(to, filename, NullS);
+ return -1;
+ }
#else
my_load_path(to, filename, NullS);
+#endif
return 0;
-#endif
}
diff --git a/sql/item_strfunc.cc b/sql/item_strfunc.cc
index 6f697a1665a..8f9a04329d3 100644
--- a/sql/item_strfunc.cc
+++ b/sql/item_strfunc.cc
@@ -2843,8 +2843,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)))
diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h
index d88e629b91b..a811bbafdb6 100644
--- a/sql/mysql_priv.h
+++ b/sql/mysql_priv.h
@@ -1264,6 +1264,8 @@ bool init_errmessage(void);
bool fn_format_relative_to_data_home(my_string to, const char *name,
const char *dir, const char *extension);
+bool is_secure_file_path(char *path);
+
File open_binlog(IO_CACHE *log, const char *log_file_name,
const char **errmsg);
diff --git a/sql/mysqld.cc b/sql/mysqld.cc
index f026bab1c32..3291085f380 100644
--- a/sql/mysqld.cc
+++ b/sql/mysqld.cc
@@ -7855,6 +7855,64 @@ fn_format_relative_to_data_home(my_string 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];
+ size_t opt_secure_file_priv_len;
+ /*
+ All paths are secure if opt_secure_file_path is 0
+ */
+ if (!opt_secure_file_priv)
+ return TRUE;
+
+ opt_secure_file_priv_len= strlen(opt_secure_file_priv);
+
+ if (strlen(path) >= FN_REFLEN)
+ return FALSE;
+
+ 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 (!lower_case_file_system)
+ {
+ if (strncmp(opt_secure_file_priv, buff2, opt_secure_file_priv_len))
+ return FALSE;
+ }
+ else
+ {
+ if (files_charset_info->coll->strnncoll(files_charset_info,
+ (uchar *) buff2, strlen(buff2),
+ (uchar *) opt_secure_file_priv,
+ opt_secure_file_priv_len,
+ TRUE))
+ return FALSE;
+ }
+ return TRUE;
+}
+
+
static void fix_paths(void)
{
char buff[FN_REFLEN],*pos;
diff --git a/sql/sql_class.cc b/sql/sql_class.cc
index 06f2229a050..cd2f2029ca2 100644
--- a/sql/sql_class.cc
+++ b/sql/sql_class.cc
@@ -1211,8 +1211,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");
diff --git a/sql/sql_load.cc b/sql/sql_load.cc
index 83af6d477db..9cead8c0ff1 100644
--- a/sql/sql_load.cc
+++ b/sql/sql_load.cc
@@ -287,36 +287,36 @@ int mysql_load(THD *thd,sql_exchange *ex,TABLE_LIST *table_list,
{
(void) fn_format(name, ex->file_name, mysql_real_data_home, "",
MY_RELATIVE_PATH | MY_UNPACK_FILENAME);
+ }
+
+ if (!is_secure_file_path(name))
+ {
+ /* 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 !defined(__WIN__) && !defined(OS2) && ! defined(__NETWARE__)
- MY_STAT stat_info;
- if (!my_stat(name,&stat_info,MYF(MY_WME)))
- DBUG_RETURN(TRUE);
+ MY_STAT stat_info;
+ if (!my_stat(name, &stat_info, MYF(MY_WME)))
+ DBUG_RETURN(TRUE);
- // if we are not in slave thread, the file must be:
- if (!thd->slave_thread &&
- !((stat_info.st_mode & S_IROTH) == S_IROTH && // readable by others
+ // if we are not in slave thread, the file must be:
+ if (!thd->slave_thread &&
+ !((stat_info.st_mode & S_IROTH) == S_IROTH && // readable by others
#ifndef __EMX__
- (stat_info.st_mode & S_IFLNK) != S_IFLNK && // and not a symlink
+ (stat_info.st_mode & S_IFLNK) != S_IFLNK && // and not a symlink
#endif
- ((stat_info.st_mode & S_IFREG) == S_IFREG ||
- (stat_info.st_mode & S_IFIFO) == S_IFIFO)))
- {
- my_error(ER_TEXTFILE_NOT_READABLE, MYF(0), name);
- DBUG_RETURN(TRUE);
- }
- if ((stat_info.st_mode & S_IFIFO) == S_IFIFO)
- is_fifo = 1;
+ ((stat_info.st_mode & S_IFREG) == S_IFREG || // and a regular file
+ (stat_info.st_mode & S_IFIFO) == S_IFIFO))) // or FIFO
+ {
+ my_error(ER_TEXTFILE_NOT_READABLE, MYF(0), name);
+ DBUG_RETURN(TRUE);
+ }
+ 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);
}