From 93721fdd94f90d48b290749398a26cef277ad129 Mon Sep 17 00:00:00 2001 From: Ben Fuhrmannek Date: Tue, 24 Jun 2014 16:56:21 +0200 Subject: Added SQL injection protection for Mysqli and several test cases --- execute.c | 128 ++++++++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 83 insertions(+), 45 deletions(-) (limited to 'execute.c') diff --git a/execute.c b/execute.c index 1f7cf15..098b074 100644 --- a/execute.c +++ b/execute.c @@ -880,7 +880,7 @@ int ih_querycheck(IH_HANDLER_PARAMS) return (0); } - if ((long) ih->arg1) { + if ((long) ih->arg2) { mysql_extension = 1; } @@ -892,6 +892,7 @@ int ih_querycheck(IH_HANDLER_PARAMS) } len = Z_STRLEN_P(backup); query = Z_STRVAL_P(backup); + SDEBUG("SQL |%s|", query); s = query; e = s+len; @@ -1552,9 +1553,9 @@ static int ih_getrandmax(IH_HANDLER_PARAMS) } internal_function_handler ihandlers[] = { - { "preg_replace", ih_preg_replace, NULL, NULL, NULL }, - { "mail", ih_mail, NULL, NULL, NULL }, - { "symlink", ih_symlink, NULL, NULL, NULL }, + { "preg_replace", ih_preg_replace, NULL, NULL, NULL }, + { "mail", ih_mail, NULL, NULL, NULL }, + { "symlink", ih_symlink, NULL, NULL, NULL }, { "srand", ih_srand, NULL, NULL, NULL }, { "mt_srand", ih_mt_srand, NULL, NULL, NULL }, @@ -1563,49 +1564,86 @@ internal_function_handler ihandlers[] = { { "getrandmax", ih_getrandmax, NULL, NULL, NULL }, { "mt_getrandmax", ih_getrandmax, NULL, NULL, NULL }, - { "ocilogon", ih_fixusername, (void *)1, NULL, NULL }, - { "ociplogon", ih_fixusername, (void *)1, NULL, NULL }, - { "ocinlogon", ih_fixusername, (void *)1, NULL, NULL }, - { "oci_connect", ih_fixusername, (void *)1, NULL, NULL }, - { "oci_pconnect", ih_fixusername, (void *)1, NULL, NULL }, - { "oci_new_connect", ih_fixusername, (void *)1, NULL, NULL }, + { "function_exists", ih_function_exists, NULL, NULL, NULL }, - { "fbsql_change_user", ih_fixusername, (void *)1, NULL, NULL }, - { "fbsql_connect", ih_fixusername, (void *)2, NULL, NULL }, - { "fbsql_pconnect", ih_fixusername, (void *)2, NULL, NULL }, - - { "function_exists", ih_function_exists, NULL, NULL, NULL }, + /* Mysqli */ + { "mysqli::mysqli", ih_fixusername, (void *)2, NULL, NULL }, + { "mysqli_connect", ih_fixusername, (void *)2, NULL, NULL }, + { "mysqli::real_connect", ih_fixusername, (void *)2, NULL, NULL }, + { "mysqli_real_connect", ih_fixusername, (void *)3, NULL, NULL }, + { "mysqli_change_user", ih_fixusername, (void *)2, NULL, NULL }, + { "mysqli::change_user", ih_fixusername, (void *)1, NULL, NULL }, + + { "mysqli::query", ih_querycheck, (void *)1, (void *)1, NULL }, + { "mysqli_query", ih_querycheck, (void *)2, (void *)1, NULL }, + { "mysqli::multi_query", ih_querycheck, (void *)1, (void *)1, NULL }, + { "mysqli_multi_query", ih_querycheck, (void *)2, (void *)1, NULL }, + { "mysqli::prepare", ih_querycheck, (void *)1, (void *)1, NULL }, + { "mysqli_prepare", ih_querycheck, (void *)2, (void *)1, NULL }, + { "mysqli::real_query", ih_querycheck, (void *)1, (void *)1, NULL }, + { "mysqli_real_query", ih_querycheck, (void *)2, (void *)1, NULL }, + { "mysqli::send_query", ih_querycheck, (void *)1, (void *)1, NULL }, + { "mysqli_send_query", ih_querycheck, (void *)2, (void *)1, NULL }, + // removed in PHP 5.3 + { "mysqli_master_query", ih_querycheck, (void *)2, (void *)1, NULL }, + { "mysqli_slave_query", ih_querycheck, (void *)2, (void *)1, NULL }, + // ---- + + /* Mysql API - deprecated in PHP 5.5 */ + { "mysql_connect", ih_fixusername, (void *)2, NULL, NULL }, + { "mysql_pconnect", ih_fixusername, (void *)2, NULL, NULL }, + { "mysql_query", ih_querycheck, (void *)1, (void *)1, NULL }, + { "mysql_db_query", ih_querycheck, (void *)2, (void *)1, NULL }, + { "mysql_unbuffered_query", ih_querycheck, (void *)1, (void *)1, NULL }, + + /* MaxDB */ + { "maxdb::maxdb", ih_fixusername, (void *)2, NULL, NULL }, + { "maxdb_connect", ih_fixusername, (void *)2, NULL, NULL }, + { "maxdb::real_connect", ih_fixusername, (void *)2, NULL, NULL }, + { "maxdb_real_connect", ih_fixusername, (void *)3, NULL, NULL }, + { "maxdb::change_user", ih_fixusername, (void *)1, NULL, NULL }, + { "maxdb_change_user", ih_fixusername, (void *)2, NULL, NULL }, + + { "maxdb_master_query", ih_querycheck, (void *)2, NULL, NULL }, + { "maxdb::multi_query", ih_querycheck, (void *)1, NULL, NULL }, + { "maxdb_multi_query", ih_querycheck, (void *)2, NULL, NULL }, + { "maxdb::query", ih_querycheck, (void *)1, NULL, NULL }, + { "maxdb_query", ih_querycheck, (void *)2, NULL, NULL }, + { "maxdb::real_query", ih_querycheck, (void *)1, NULL, NULL }, + { "maxdb_real_query", ih_querycheck, (void *)2, NULL, NULL }, + { "maxdb::send_query", ih_querycheck, (void *)1, NULL, NULL }, + { "maxdb_send_query", ih_querycheck, (void *)2, NULL, NULL }, + { "maxdb::prepare", ih_querycheck, (void *)1, NULL, NULL }, + { "maxdb_prepare", ih_querycheck, (void *)2, NULL, NULL }, + + /* Oracle OCI8 */ + { "ocilogon", ih_fixusername, (void *)1, NULL, NULL }, + { "ociplogon", ih_fixusername, (void *)1, NULL, NULL }, + { "ocinlogon", ih_fixusername, (void *)1, NULL, NULL }, + { "oci_connect", ih_fixusername, (void *)1, NULL, NULL }, + { "oci_pconnect", ih_fixusername, (void *)1, NULL, NULL }, + { "oci_new_connect", ih_fixusername, (void *)1, NULL, NULL }, + + /* FrontBase */ + { "fbsql_connect", ih_fixusername, (void *)2, NULL, NULL }, + { "fbsql_pconnect", ih_fixusername, (void *)2, NULL, NULL }, + { "fbsql_change_user", ih_fixusername, (void *)1, NULL, NULL }, + { "fbsql_username", ih_fixusername, (void *)2, NULL, NULL }, + + /* Informix */ + { "ifx_connect", ih_fixusername, (void *)2, NULL, NULL }, + { "ifx_pconnect", ih_fixusername, (void *)2, NULL, NULL }, + + /* Firebird/InterBase */ + { "ibase_connect", ih_fixusername, (void *)2, NULL, NULL }, + { "ibase_pconnect", ih_fixusername, (void *)2, NULL, NULL }, + { "ibase_service_attach", ih_fixusername, (void *)2, NULL, NULL }, + + /* Microsoft SQL Server */ + { "mssql_connect", ih_fixusername, (void *)2, NULL, NULL }, + { "mssql_pconnect", ih_fixusername, (void *)2, NULL, NULL }, - { "ifx_connect", ih_fixusername, (void *)2, NULL, NULL }, - { "ifx_pconnect", ih_fixusername, (void *)2, NULL, NULL }, - - { "ibase_connect", ih_fixusername, (void *)2, NULL, NULL }, - { "ibase_pconnect", ih_fixusername, (void *)2, NULL, NULL }, - - { "maxdb", ih_fixusername, (void *)2, NULL, NULL }, - { "maxdb_change_user", ih_fixusername, (void *)2, NULL, NULL }, - { "maxdb_connect", ih_fixusername, (void *)2, NULL, NULL }, - { "maxdb_pconnect", ih_fixusername, (void *)2, NULL, NULL }, - { "maxdb_real_connect", ih_fixusername, (void *)3, NULL, NULL }, - - { "mssql_connect", ih_fixusername, (void *)2, NULL, NULL }, - { "mssql_pconnect", ih_fixusername, (void *)2, NULL, NULL }, - - { "mysql_query", ih_querycheck, (void *)1, (void *)1, NULL }, - { "mysql_db_query", ih_querycheck, (void *)2, (void *)1, NULL }, - { "mysql_unbuffered_query", ih_querycheck, (void *)1, (void *)1, NULL }, - { "mysqli_query", ih_querycheck, (void *)2, (void *)1, NULL }, - { "mysqli_real_query", ih_querycheck, (void *)2, (void *)1, NULL }, - { "mysqli_send_query", ih_querycheck, (void *)2, (void *)1, NULL }, - { "mysqli_master_query", ih_querycheck, (void *)2, (void *)1, NULL }, - { "mysqli_slave_query", ih_querycheck, (void *)2, (void *)1, NULL }, - - { "mysqli", ih_fixusername, (void *)2, NULL, NULL }, - { "mysql_connect", ih_fixusername, (void *)2, NULL, NULL }, - { "mysql_pconnect", ih_fixusername, (void *)2, NULL, NULL }, - { "mysqli_change_user", ih_fixusername, (void *)2, NULL, NULL }, - { "mysql_real_connect", ih_fixusername, (void *)3, NULL, NULL }, - { NULL, NULL, NULL, NULL, NULL } + { NULL, NULL, NULL, NULL, NULL } }; #define FUNCTION_WARNING() zend_error(E_WARNING, "%s() has been disabled for security reasons", get_active_function_name(TSRMLS_C)); -- cgit v1.3 From 63de1053dfda1faca22a84afb82d6b1315b8db6e Mon Sep 17 00:00:00 2001 From: Ben Fuhrmannek Date: Wed, 9 Jul 2014 12:47:03 +0200 Subject: added sql.user_match + username character check --- Changelog | 4 ++- execute.c | 51 ++++++++++++++++++++++++---------- php_suhosin.h | 45 +++++++++++++++--------------- suhosin.c | 15 +++++----- tests/sql/mysqli_user_match_error.phpt | 18 ++++++++++++ tests/sql/mysqli_user_match_ok.phpt | 18 ++++++++++++ 6 files changed, 106 insertions(+), 45 deletions(-) create mode 100644 tests/sql/mysqli_user_match_error.phpt create mode 100644 tests/sql/mysqli_user_match_ok.phpt (limited to 'execute.c') diff --git a/Changelog b/Changelog index 4e83cb3..ad2438f 100644 --- a/Changelog +++ b/Changelog @@ -1,6 +1,8 @@ 2014-06-24 - 0.9.37-dev - - Added SQL injection protection for Mysqli and several test cases + - Added SQL injection protection for Mysqli and several test cases + - Added wildcard matching for SQL username + - Added check for SQL username to only contain valid characters (>= ASCII 32) 2014-06-10 - 0.9.36 diff --git a/execute.c b/execute.c index 098b074..2f280b7 100644 --- a/execute.c +++ b/execute.c @@ -24,6 +24,7 @@ #endif #include +#include #include "php.h" #include "php_ini.h" #include "zend_hash.h" @@ -1024,17 +1025,20 @@ int ih_fixusername(IH_HANDLER_PARAMS) void **p = EG(argument_stack).top_element-2; #endif unsigned long arg_count; - zval **arg;char *prefix, *postfix, *user; + zval **arg; + char *prefix, *postfix, *user, *user_match, *cp; zval *backup, *my_user; int prefix_len, postfix_len, len; - SDEBUG("function: %s", ih->name); + SDEBUG("function (fixusername): %s", ih->name); prefix = SUHOSIN_G(sql_user_prefix); postfix = SUHOSIN_G(sql_user_postfix); + user_match = SUHOSIN_G(sql_user_match); - if ((prefix == NULL || prefix[0] == 0)&& - (postfix == NULL || postfix[0] == 0)) { + if ((prefix == NULL || prefix[0] == 0) && + (postfix == NULL || postfix[0] == 0) && + (user_match == NULL || user_match[0] == 0)) { return (0); } @@ -1065,23 +1069,40 @@ int ih_fixusername(IH_HANDLER_PARAMS) user = Z_STRVAL_P(backup); } - if (prefix_len && prefix_len <= len) { - if (strncmp(prefix, user, prefix_len)==0) { - prefix = ""; - len -= prefix_len; - } - } - - if (postfix_len && postfix_len <= len) { - if (strncmp(postfix, user+len-postfix_len, postfix_len)==0) { - postfix = ""; + cp = user; + while (cp < user+len) { + if (*cp < 32) { + suhosin_log(S_SQL, "SQL username contains invalid characters"); + if (!SUHOSIN_G(simulation)) { + suhosin_bailout(TSRMLS_C); + } } + cp++; } - + MAKE_STD_ZVAL(my_user); my_user->type = IS_STRING; my_user->value.str.len = spprintf(&my_user->value.str.val, 0, "%s%s%s", prefix, user, postfix); + if (user_match && user_match[0]) { + len = Z_STRLEN_P(my_user); + user = Z_STRVAL_P(my_user); +#ifdef HAVE_FNMATCH + if (fnmatch(user_match, user, 0) != 0) { + suhosin_log(S_SQL, "SQL username ('%s') does not match suhosin.sql.user_match ('%s')", user, user_match); + if (!SUHOSIN_G(simulation)) { + suhosin_bailout(TSRMLS_C); + } + } +#else +#warning no support for fnmatch() - setting suhosin.sql.user_match will always fail. + suhosin_log(S_SQL, "suhosin.sql.user_match specified, but system does not support fnmatch()"); + if (!SUHOSIN_G(simulation)) { + suhosin_bailout(TSRMLS_C); + } +#endif + } + /* XXX: memory_leak? */ *arg = my_user; diff --git a/php_suhosin.h b/php_suhosin.h index e5604a7..e85fc38 100644 --- a/php_suhosin.h +++ b/php_suhosin.h @@ -76,15 +76,16 @@ ZEND_BEGIN_MODULE_GLOBALS(suhosin) char *filter_action; char *sql_user_prefix; char *sql_user_postfix; - long sql_comment; - long sql_opencomment; - long sql_union; - long sql_mselect; + char *sql_user_match; + long sql_comment; + long sql_opencomment; + long sql_union; + long sql_mselect; long max_execution_depth; zend_bool abort_request; long executor_include_max_traversal; - zend_bool executor_include_allow_writable_files; + zend_bool executor_include_allow_writable_files; HashTable *include_whitelist; @@ -152,11 +153,11 @@ ZEND_BEGIN_MODULE_GLOBALS(suhosin) zend_bool upload_remove_binary; char *upload_verification_script; - zend_bool no_more_variables; - zend_bool no_more_get_variables; - zend_bool no_more_post_variables; - zend_bool no_more_cookie_variables; - zend_bool no_more_uploads; + zend_bool no_more_variables; + zend_bool no_more_get_variables; + zend_bool no_more_post_variables; + zend_bool no_more_cookie_variables; + zend_bool no_more_uploads; @@ -198,8 +199,8 @@ ZEND_BEGIN_MODULE_GLOBALS(suhosin) int (*old_s_destroy)(void **mod_data, const char *key TSRMLS_DC); BYTE fi[24],ri[24]; - WORD fkey[120]; - WORD rkey[120]; + WORD fkey[120]; + WORD rkey[120]; zend_bool session_encrypt; char* session_cryptkey; @@ -247,16 +248,16 @@ ZEND_BEGIN_MODULE_GLOBALS(suhosin) zend_bool mt_is_seeded; /* PERDIR Handling */ - char *perdir; - zend_bool log_perdir; - zend_bool exec_perdir; - zend_bool get_perdir; - zend_bool post_perdir; - zend_bool cookie_perdir; - zend_bool request_perdir; - zend_bool upload_perdir; - zend_bool sql_perdir; - zend_bool misc_perdir; + char *perdir; + zend_bool log_perdir; + zend_bool exec_perdir; + zend_bool get_perdir; + zend_bool post_perdir; + zend_bool cookie_perdir; + zend_bool request_perdir; + zend_bool upload_perdir; + zend_bool sql_perdir; + zend_bool misc_perdir; ZEND_END_MODULE_GLOBALS(suhosin) diff --git a/suhosin.c b/suhosin.c index 0d1eba0..469e88f 100644 --- a/suhosin.c +++ b/suhosin.c @@ -989,13 +989,14 @@ PHP_INI_BEGIN() STD_PHP_INI_ENTRY("suhosin.upload.verification_script", NULL, PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateUploadString, upload_verification_script, zend_suhosin_globals, suhosin_globals) - STD_ZEND_INI_BOOLEAN("suhosin.sql.bailout_on_error", "0", ZEND_INI_PERDIR|ZEND_INI_SYSTEM, OnUpdateSQLBool, sql_bailout_on_error, zend_suhosin_globals, suhosin_globals) - STD_PHP_INI_ENTRY("suhosin.sql.user_prefix", NULL, PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateSQLString, sql_user_prefix, zend_suhosin_globals, suhosin_globals) - STD_PHP_INI_ENTRY("suhosin.sql.user_postfix", NULL, PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateSQLString, sql_user_postfix, zend_suhosin_globals, suhosin_globals) - STD_PHP_INI_ENTRY("suhosin.sql.comment", "0", PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateSQLLong, sql_comment, zend_suhosin_globals, suhosin_globals) - STD_PHP_INI_ENTRY("suhosin.sql.opencomment", "0", PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateSQLLong, sql_opencomment, zend_suhosin_globals, suhosin_globals) - STD_PHP_INI_ENTRY("suhosin.sql.multiselect", "0", PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateSQLLong, sql_mselect, zend_suhosin_globals, suhosin_globals) - STD_PHP_INI_ENTRY("suhosin.sql.union", "0", PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateSQLLong, sql_union, zend_suhosin_globals, suhosin_globals) + STD_ZEND_INI_BOOLEAN("suhosin.sql.bailout_on_error", "0", ZEND_INI_PERDIR|ZEND_INI_SYSTEM, OnUpdateSQLBool, sql_bailout_on_error, zend_suhosin_globals, suhosin_globals) + STD_PHP_INI_ENTRY("suhosin.sql.user_prefix", NULL, PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateSQLString, sql_user_prefix, zend_suhosin_globals, suhosin_globals) + STD_PHP_INI_ENTRY("suhosin.sql.user_postfix", NULL, PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateSQLString, sql_user_postfix, zend_suhosin_globals, suhosin_globals) + STD_PHP_INI_ENTRY("suhosin.sql.user_match", NULL, PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateSQLString, sql_user_match, zend_suhosin_globals, suhosin_globals) + STD_PHP_INI_ENTRY("suhosin.sql.comment", "0", PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateSQLLong, sql_comment, zend_suhosin_globals, suhosin_globals) + STD_PHP_INI_ENTRY("suhosin.sql.opencomment", "0", PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateSQLLong, sql_opencomment, zend_suhosin_globals, suhosin_globals) + STD_PHP_INI_ENTRY("suhosin.sql.multiselect", "0", PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateSQLLong, sql_mselect, zend_suhosin_globals, suhosin_globals) + STD_PHP_INI_ENTRY("suhosin.sql.union", "0", PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateSQLLong, sql_union, zend_suhosin_globals, suhosin_globals) STD_ZEND_INI_BOOLEAN("suhosin.session.encrypt", "1", ZEND_INI_PERDIR|ZEND_INI_SYSTEM, OnUpdateBool, session_encrypt, zend_suhosin_globals, suhosin_globals) STD_PHP_INI_ENTRY("suhosin.session.cryptkey", "", PHP_INI_ALL, OnUpdateString, session_cryptkey, zend_suhosin_globals, suhosin_globals) diff --git a/tests/sql/mysqli_user_match_error.phpt b/tests/sql/mysqli_user_match_error.phpt new file mode 100644 index 0000000..69db081 --- /dev/null +++ b/tests/sql/mysqli_user_match_error.phpt @@ -0,0 +1,18 @@ +--TEST-- +Mysqli connect with user_match not matching username +--INI-- +extension=mysqli.so +suhosin.sql.user_match=complicated_userprefix* +suhosin.log.stdout=32 +--SKIPIF-- + +--FILE-- + +--EXPECTREGEX-- +ALERT - SQL username .* does not match.* \ No newline at end of file diff --git a/tests/sql/mysqli_user_match_ok.phpt b/tests/sql/mysqli_user_match_ok.phpt new file mode 100644 index 0000000..4d7a438 --- /dev/null +++ b/tests/sql/mysqli_user_match_ok.phpt @@ -0,0 +1,18 @@ +--TEST-- +Mysqli connect with user_match not matching username +--INI-- +extension=mysqli.so +suhosin.sql.user_match=invalid_* +suhosin.log.stdout=32 +--SKIPIF-- + +--FILE-- + +--EXPECTREGEX-- +.*Access denied for user 'invalid_username'.* \ No newline at end of file -- cgit v1.3 From 84996270798fccffe2da890ad7a7c270d298a6e8 Mon Sep 17 00:00:00 2001 From: Ben Fuhrmannek Date: Wed, 9 Jul 2014 13:55:58 +0200 Subject: enforce SQL username check + return FALSE instead of bailout --- execute.c | 51 +++++++++++++------------- tests/sql/mysqli_connect_invalid_username.phpt | 17 +++++++++ tests/sql/mysqli_user_match_ok.phpt | 2 +- 3 files changed, 43 insertions(+), 27 deletions(-) create mode 100644 tests/sql/mysqli_connect_invalid_username.phpt (limited to 'execute.c') diff --git a/execute.c b/execute.c index 2f280b7..913a82b 100644 --- a/execute.c +++ b/execute.c @@ -1036,22 +1036,6 @@ int ih_fixusername(IH_HANDLER_PARAMS) postfix = SUHOSIN_G(sql_user_postfix); user_match = SUHOSIN_G(sql_user_match); - if ((prefix == NULL || prefix[0] == 0) && - (postfix == NULL || postfix[0] == 0) && - (user_match == NULL || user_match[0] == 0)) { - return (0); - } - - if (prefix == NULL) { - prefix = ""; - } - if (postfix == NULL) { - postfix = ""; - } - - prefix_len = strlen(prefix); - postfix_len = strlen(postfix); - arg_count = (unsigned long) *p; if (ht < (long) ih->arg1) { @@ -1074,38 +1058,53 @@ int ih_fixusername(IH_HANDLER_PARAMS) if (*cp < 32) { suhosin_log(S_SQL, "SQL username contains invalid characters"); if (!SUHOSIN_G(simulation)) { - suhosin_bailout(TSRMLS_C); + RETVAL_FALSE; + return (1); } } cp++; } - MAKE_STD_ZVAL(my_user); - my_user->type = IS_STRING; - my_user->value.str.len = spprintf(&my_user->value.str.val, 0, "%s%s%s", prefix, user, postfix); + if ((prefix != NULL && prefix[0]) || (postfix != NULL && postfix[0])) { + if (prefix == NULL) { + prefix = ""; + } + if (postfix == NULL) { + postfix = ""; + } + prefix_len = strlen(prefix); + postfix_len = strlen(postfix); + + MAKE_STD_ZVAL(my_user); + my_user->type = IS_STRING; + my_user->value.str.len = spprintf(&my_user->value.str.val, 0, "%s%s%s", prefix, user, postfix); - if (user_match && user_match[0]) { + /* XXX: memory_leak? */ + *arg = my_user; + len = Z_STRLEN_P(my_user); user = Z_STRVAL_P(my_user); + } + + if (user_match && user_match[0]) { #ifdef HAVE_FNMATCH if (fnmatch(user_match, user, 0) != 0) { suhosin_log(S_SQL, "SQL username ('%s') does not match suhosin.sql.user_match ('%s')", user, user_match); if (!SUHOSIN_G(simulation)) { - suhosin_bailout(TSRMLS_C); + RETVAL_FALSE; + return (1); } } #else #warning no support for fnmatch() - setting suhosin.sql.user_match will always fail. suhosin_log(S_SQL, "suhosin.sql.user_match specified, but system does not support fnmatch()"); if (!SUHOSIN_G(simulation)) { - suhosin_bailout(TSRMLS_C); + RETVAL_FALSE; + return (1); } #endif } - /* XXX: memory_leak? */ - *arg = my_user; - SDEBUG("function: %s - user: %s", ih->name, user); return (0); diff --git a/tests/sql/mysqli_connect_invalid_username.phpt b/tests/sql/mysqli_connect_invalid_username.phpt new file mode 100644 index 0000000..532254f --- /dev/null +++ b/tests/sql/mysqli_connect_invalid_username.phpt @@ -0,0 +1,17 @@ +--TEST-- +Mysqli connect with user_match not matching username +--INI-- +extension=mysqli.so +suhosin.log.stdout=32 +--SKIPIF-- + +--FILE-- + +--EXPECTREGEX-- +ALERT - SQL username contains invalid characters.* \ No newline at end of file diff --git a/tests/sql/mysqli_user_match_ok.phpt b/tests/sql/mysqli_user_match_ok.phpt index 4d7a438..a2ad832 100644 --- a/tests/sql/mysqli_user_match_ok.phpt +++ b/tests/sql/mysqli_user_match_ok.phpt @@ -1,5 +1,5 @@ --TEST-- -Mysqli connect with user_match not matching username +Mysqli connect with user_match matching username --INI-- extension=mysqli.so suhosin.sql.user_match=invalid_* -- cgit v1.3 From e10330a298e2da81d3733e8561e5715e695feeaa Mon Sep 17 00:00:00 2001 From: Ben Fuhrmannek Date: Wed, 9 Jul 2014 14:49:06 +0200 Subject: SQL username check in sim. mode checks only for the first occ. of inv. chars --- execute.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'execute.c') diff --git a/execute.c b/execute.c index 913a82b..e73d19b 100644 --- a/execute.c +++ b/execute.c @@ -1060,6 +1060,8 @@ int ih_fixusername(IH_HANDLER_PARAMS) if (!SUHOSIN_G(simulation)) { RETVAL_FALSE; return (1); + } else { + break; } } cp++; -- cgit v1.3 From 476300ee97276020cc166066b19fc8026142f023 Mon Sep 17 00:00:00 2001 From: Ben Fuhrmannek Date: Wed, 9 Jul 2014 15:58:38 +0200 Subject: experimental PDO support --- execute.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'execute.c') diff --git a/execute.c b/execute.c index e73d19b..f2877d4 100644 --- a/execute.c +++ b/execute.c @@ -1638,6 +1638,13 @@ internal_function_handler ihandlers[] = { { "maxdb::prepare", ih_querycheck, (void *)1, NULL, NULL }, { "maxdb_prepare", ih_querycheck, (void *)2, NULL, NULL }, + /* PDO */ + /* note: mysql conditional comments not supported here */ + { "pdo::__construct", ih_fixusername, (void *)2, NULL, NULL }, /* note: username may come from dsn (param 1) */ + { "pdo::query", ih_querycheck, (void *)1, NULL, NULL }, + { "pdo::prepare", ih_querycheck, (void *)1, NULL, NULL }, + { "pdo::exec", ih_querycheck, (void *)1, NULL, NULL }, + /* Oracle OCI8 */ { "ocilogon", ih_fixusername, (void *)1, NULL, NULL }, { "ociplogon", ih_fixusername, (void *)1, NULL, NULL }, -- cgit v1.3 From 5abf81e9ddc7e70d5c223ebf906a0f29145892a9 Mon Sep 17 00:00:00 2001 From: Ben Fuhrmannek Date: Wed, 9 Jul 2014 16:29:48 +0200 Subject: untested features must be enabled: configure --enable-suhosin-experimental --- .gitignore | 9 ++++++++- config.m4 | 7 +++++++ execute.c | 4 +++- 3 files changed, 18 insertions(+), 2 deletions(-) (limited to 'execute.c') diff --git a/.gitignore b/.gitignore index a2e673f..cd5d051 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,12 @@ .DS_Store -/config.* +/config.log +config.guess +config.h +config.h.in +config.h.in~ +config.nice +config.status +config.sub /*.lo /.deps /.libs/ diff --git a/config.m4 b/config.m4 index ee4b7ba..01edafd 100644 --- a/config.m4 +++ b/config.m4 @@ -7,3 +7,10 @@ PHP_ARG_ENABLE(suhosin, whether to enable suhosin support, if test "$PHP_SUHOSIN" != "no"; then PHP_NEW_EXTENSION(suhosin, suhosin.c sha256.c memory_limit.c treat_data.c ifilter.c post_handler.c ufilter.c rfc1867.c rfc1867_new.c log.c header.c execute.c ex_imp.c session.c aes.c compat_snprintf.c, $ext_shared) fi + +PHP_ARG_ENABLE(suhosin-experimental, whether to enable experimental suhosin features, +[ --enable-suhosin-experimental Enable experimental suhosin features], no, no) + +if test "$PHP_SUHOSIN_EXPERIMENTAL" != "no"; then + AC_DEFINE(SUHOSIN_EXPERIMENTAL, 1, [Whether to enable experimental suhosin features]) +fi diff --git a/execute.c b/execute.c index f2877d4..82a4866 100644 --- a/execute.c +++ b/execute.c @@ -1618,6 +1618,7 @@ internal_function_handler ihandlers[] = { { "mysql_db_query", ih_querycheck, (void *)2, (void *)1, NULL }, { "mysql_unbuffered_query", ih_querycheck, (void *)1, (void *)1, NULL }, +#ifdef SUHOSIN_EXPERIMENTAL /* MaxDB */ { "maxdb::maxdb", ih_fixusername, (void *)2, NULL, NULL }, { "maxdb_connect", ih_fixusername, (void *)2, NULL, NULL }, @@ -1671,7 +1672,8 @@ internal_function_handler ihandlers[] = { /* Microsoft SQL Server */ { "mssql_connect", ih_fixusername, (void *)2, NULL, NULL }, { "mssql_pconnect", ih_fixusername, (void *)2, NULL, NULL }, - +#endif + { NULL, NULL, NULL, NULL, NULL } }; -- cgit v1.3