Commit 91a4a8ab authored by Mats Kindahl's avatar Mats Kindahl

BUG#58246: INSTALL PLUGIN not secure & crashable

When installing plugins, there is a missing check
for slash (/) in the path on Windows. Note that on
Windows, both / and \ can be used to separate
directories.

This patch fixes the issue by:
- Adding a FN_DIRSEP symbol for all platforms
  consisting of a string of legal directory
  separators.
- Adding a charset-aware version of strcspn().
- Adding a check_valid_path() function that uses
  my_strcspn() to check if any FN_DIRSEP character
  is in the supplied string.
- Using the check_valid_path() function in
  sql_plugin.cc and sql_udf.cc (which means
  replacing the existing test there).
parent 80246ac8
...@@ -122,6 +122,7 @@ extern "C" { ...@@ -122,6 +122,7 @@ extern "C" {
#define CANT_DELETE_OPEN_FILES 1 #define CANT_DELETE_OPEN_FILES 1
#define FN_LIBCHAR '\\' #define FN_LIBCHAR '\\'
#define FN_DIRSEP "/\\" /* Valid directory separators */
#define FN_ROOTDIR "\\" #define FN_ROOTDIR "\\"
#define FN_DEVCHAR ':' #define FN_DEVCHAR ':'
......
...@@ -332,6 +332,7 @@ inline ulonglong double2ulonglong(double d) ...@@ -332,6 +332,7 @@ inline ulonglong double2ulonglong(double d)
/* File name handling */ /* File name handling */
#define FN_LIBCHAR '\\' #define FN_LIBCHAR '\\'
#define FN_DIRSEP "/\\" /* Valid directory separators */
#define FN_ROOTDIR "\\" #define FN_ROOTDIR "\\"
#define FN_DEVCHAR ':' #define FN_DEVCHAR ':'
#define FN_NETWORK_DRIVES /* Uses \\ to indicate network drives */ #define FN_NETWORK_DRIVES /* Uses \\ to indicate network drives */
......
...@@ -464,6 +464,8 @@ extern my_bool my_parse_charset_xml(const char *bug, size_t len, ...@@ -464,6 +464,8 @@ extern my_bool my_parse_charset_xml(const char *bug, size_t len,
int (*add)(CHARSET_INFO *cs)); int (*add)(CHARSET_INFO *cs));
extern char *my_strchr(CHARSET_INFO *cs, const char *str, const char *end, extern char *my_strchr(CHARSET_INFO *cs, const char *str, const char *end,
pchar c); pchar c);
extern size_t my_strcspn(CHARSET_INFO *cs, const char *str, const char *end,
const char *accept);
my_bool my_propagate_simple(CHARSET_INFO *cs, const uchar *str, size_t len); my_bool my_propagate_simple(CHARSET_INFO *cs, const uchar *str, size_t len);
my_bool my_propagate_complex(CHARSET_INFO *cs, const uchar *str, size_t len); my_bool my_propagate_complex(CHARSET_INFO *cs, const uchar *str, size_t len);
......
...@@ -758,6 +758,7 @@ typedef SOCKET_SIZE_TYPE size_socket; ...@@ -758,6 +758,7 @@ typedef SOCKET_SIZE_TYPE size_socket;
#ifndef FN_LIBCHAR #ifndef FN_LIBCHAR
#define FN_LIBCHAR '/' #define FN_LIBCHAR '/'
#define FN_DIRSEP "/" /* Valid directory separators */
#define FN_ROOTDIR "/" #define FN_ROOTDIR "/"
#endif #endif
#define MY_NFILE 64 /* This is only used to save filenames */ #define MY_NFILE 64 /* This is only used to save filenames */
......
...@@ -8,3 +8,5 @@ ERROR 42000: DELETE command denied to user 'bug51770'@'localhost' for table 'plu ...@@ -8,3 +8,5 @@ ERROR 42000: DELETE command denied to user 'bug51770'@'localhost' for table 'plu
GRANT DELETE ON mysql.plugin TO bug51770@localhost; GRANT DELETE ON mysql.plugin TO bug51770@localhost;
UNINSTALL PLUGIN example; UNINSTALL PLUGIN example;
DROP USER bug51770@localhost; DROP USER bug51770@localhost;
INSTALL PLUGIN example SONAME '../ha_example.so';
ERROR HY000: No paths allowed for shared library
...@@ -18,3 +18,14 @@ UNINSTALL PLUGIN example; ...@@ -18,3 +18,14 @@ UNINSTALL PLUGIN example;
disconnect con1; disconnect con1;
connection default; connection default;
DROP USER bug51770@localhost; DROP USER bug51770@localhost;
#
# BUG#58246: INSTALL PLUGIN not secure & crashable
#
# The bug consisted of not recognizing / on Windows, so checking / on
# all platforms should cover this case.
let $path = `select CONCAT_WS('/', '..', $HA_EXAMPLE_SO)`;
--error ER_UDF_NO_PATHS
eval INSTALL PLUGIN example SONAME '$path';
...@@ -231,6 +231,26 @@ extern bool check_if_table_exists(THD *thd, TABLE_LIST *table, bool *exists); ...@@ -231,6 +231,26 @@ extern bool check_if_table_exists(THD *thd, TABLE_LIST *table, bool *exists);
#endif /* EMBEDDED_LIBRARY */ #endif /* EMBEDDED_LIBRARY */
/**
Check if the provided path is valid in the sense that it does cause
a relative reference outside the directory.
@note Currently, this function only check if there are any
characters in FN_DIRSEP in the string, but it might change in the
future.
@code
check_valid_path("../foo.so") -> true
check_valid_path("foo.so") -> false
@endcode
*/
bool check_valid_path(const char *path, size_t len)
{
size_t prefix= my_strcspn(files_charset_info, path, path + len, FN_DIRSEP);
return prefix < len;
}
/**************************************************************************** /****************************************************************************
Value type thunks, allows the C world to play in the C++ world Value type thunks, allows the C world to play in the C++ world
****************************************************************************/ ****************************************************************************/
...@@ -354,13 +374,14 @@ static st_plugin_dl *plugin_dl_add(const LEX_STRING *dl, int report) ...@@ -354,13 +374,14 @@ static st_plugin_dl *plugin_dl_add(const LEX_STRING *dl, int report)
struct st_plugin_dl *tmp, plugin_dl; struct st_plugin_dl *tmp, plugin_dl;
void *sym; void *sym;
DBUG_ENTER("plugin_dl_add"); DBUG_ENTER("plugin_dl_add");
DBUG_PRINT("enter", ("dl->str: '%s', dl->length: %d", dl->str, dl->length));
plugin_dir_len= strlen(opt_plugin_dir); plugin_dir_len= strlen(opt_plugin_dir);
/* /*
Ensure that the dll doesn't have a path. Ensure that the dll doesn't have a path.
This is done to ensure that only approved libraries from the This is done to ensure that only approved libraries from the
plugin directory are used (to make this even remotely secure). plugin directory are used (to make this even remotely secure).
*/ */
if (my_strchr(files_charset_info, dl->str, dl->str + dl->length, FN_LIBCHAR) || if (check_valid_path(dl->str, dl->length) ||
check_string_char_length((LEX_STRING *) dl, "", NAME_CHAR_LEN, check_string_char_length((LEX_STRING *) dl, "", NAME_CHAR_LEN,
system_charset_info, 1) || system_charset_info, 1) ||
plugin_dir_len + dl->length + 1 >= FN_REFLEN) plugin_dir_len + dl->length + 1 >= FN_REFLEN)
......
...@@ -131,6 +131,7 @@ extern bool mysql_uninstall_plugin(THD *thd, const LEX_STRING *name); ...@@ -131,6 +131,7 @@ extern bool mysql_uninstall_plugin(THD *thd, const LEX_STRING *name);
extern bool plugin_register_builtin(struct st_mysql_plugin *plugin); extern bool plugin_register_builtin(struct st_mysql_plugin *plugin);
extern void plugin_thdvar_init(THD *thd); extern void plugin_thdvar_init(THD *thd);
extern void plugin_thdvar_cleanup(THD *thd); extern void plugin_thdvar_cleanup(THD *thd);
extern bool check_valid_path(const char *path, size_t length);
typedef my_bool (plugin_foreach_func)(THD *thd, typedef my_bool (plugin_foreach_func)(THD *thd,
plugin_ref plugin, plugin_ref plugin,
......
...@@ -173,10 +173,7 @@ void udf_init() ...@@ -173,10 +173,7 @@ void udf_init()
On windows we must check both FN_LIBCHAR and '/'. On windows we must check both FN_LIBCHAR and '/'.
*/ */
if (my_strchr(files_charset_info, dl_name, if (check_valid_path(dl_name, strlen(dl_name)) ||
dl_name + strlen(dl_name), FN_LIBCHAR) ||
IF_WIN(my_strchr(files_charset_info, dl_name,
dl_name + strlen(dl_name), '/'), 0) ||
check_string_char_length(&name, "", NAME_CHAR_LEN, check_string_char_length(&name, "", NAME_CHAR_LEN,
system_charset_info, 1)) system_charset_info, 1))
{ {
...@@ -416,13 +413,8 @@ int mysql_create_function(THD *thd,udf_func *udf) ...@@ -416,13 +413,8 @@ int mysql_create_function(THD *thd,udf_func *udf)
Ensure that the .dll doesn't have a path Ensure that the .dll doesn't have a path
This is done to ensure that only approved dll from the system This is done to ensure that only approved dll from the system
directories are used (to make this even remotely secure). directories are used (to make this even remotely secure).
On windows we must check both FN_LIBCHAR and '/'.
*/ */
if (my_strchr(files_charset_info, udf->dl, if (check_valid_path(udf->dl, strlen(udf->dl)))
udf->dl + strlen(udf->dl), FN_LIBCHAR) ||
IF_WIN(my_strchr(files_charset_info, udf->dl,
udf->dl + strlen(udf->dl), '/'), 0))
{ {
my_message(ER_UDF_NO_PATHS, ER(ER_UDF_NO_PATHS), MYF(0)); my_message(ER_UDF_NO_PATHS, ER(ER_UDF_NO_PATHS), MYF(0));
DBUG_RETURN(1); DBUG_RETURN(1);
......
...@@ -13,6 +13,45 @@ ...@@ -13,6 +13,45 @@
along with this program; if not, write to the Free Software along with this program; if not, write to the Free Software
Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */
#include <my_global.h>
#include "m_string.h"
#include "m_ctype.h"
#define NEQ(A, B) ((A) != (B))
#define EQU(A, B) ((A) == (B))
/**
Macro for the body of the string scanning.
@param CS The character set of the string
@param STR Pointer to beginning of string
@param END Pointer to one-after-end of string
@param ACC Pointer to beginning of accept (or reject) string
@param LEN Length of accept (or reject) string
@param CMP is a function-like for doing the comparison of two characters.
*/
#define SCAN_STRING(CS, STR, END, ACC, LEN, CMP) \
do { \
uint mbl; \
const char *ptr_str, *ptr_acc; \
const char *acc_end= (ACC) + (LEN); \
for (ptr_str= (STR) ; ptr_str < (END) ; ptr_str+= mbl) \
{ \
mbl= my_mbcharlen((CS), *(uchar*)ptr_str); \
if (mbl < 2) \
{ \
DBUG_ASSERT(mbl == 1); \
for (ptr_acc= (ACC) ; ptr_acc < acc_end ; ++ptr_acc) \
if (CMP(*ptr_acc, *ptr_str)) \
goto end; \
} \
} \
end: \
return (size_t) (ptr_str - (STR)); \
} while (0)
/* /*
my_strchr(cs, str, end, c) returns a pointer to the first place in my_strchr(cs, str, end, c) returns a pointer to the first place in
str where c (1-byte character) occurs, or NULL if c does not occur str where c (1-byte character) occurs, or NULL if c does not occur
...@@ -21,11 +60,6 @@ ...@@ -21,11 +60,6 @@
frequently. frequently.
*/ */
#include <my_global.h>
#include "m_string.h"
#include "m_ctype.h"
char *my_strchr(CHARSET_INFO *cs, const char *str, const char *end, char *my_strchr(CHARSET_INFO *cs, const char *str, const char *end,
pchar c) pchar c)
{ {
...@@ -45,3 +79,26 @@ char *my_strchr(CHARSET_INFO *cs, const char *str, const char *end, ...@@ -45,3 +79,26 @@ char *my_strchr(CHARSET_INFO *cs, const char *str, const char *end,
return(0); return(0);
} }
/**
Calculate the length of the initial segment of 'str' which consists
entirely of characters not in 'reject'.
@note The reject string points to single-byte characters so it is
only possible to find the first occurrence of a single-byte
character. Multi-byte characters in 'str' are treated as not
matching any character in the reject string.
@todo should be moved to CHARSET_INFO if it's going to be called
frequently.
@internal The implementation builds on the assumption that 'str' is long,
while 'reject' is short. So it compares each character in string
with the characters in 'reject' in a tight loop over the characters
in 'reject'.
*/
size_t my_strcspn(CHARSET_INFO *cs, const char *str, const char *str_end,
const char *reject)
{
SCAN_STRING(cs, str, str_end, reject, strlen(reject), EQU);
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment