From 5193b37822269c19a58b86c8a6e1f8e90bd818e6 Mon Sep 17 00:00:00 2001 From: Ben Fuhrmannek Date: Thu, 17 Jul 2014 13:40:39 +0200 Subject: removed redundant implementations of protected varname check --- ex_imp.c | 59 +---------------------------------------------------------- 1 file changed, 1 insertion(+), 58 deletions(-) (limited to 'ex_imp.c') diff --git a/ex_imp.c b/ex_imp.c index 3325e43..40d40e0 100644 --- a/ex_imp.c +++ b/ex_imp.c @@ -443,6 +443,7 @@ PHP_FUNCTION(suhosin_extract) /* }}} */ + #if PHP_VERSION_ID >= 50300 static int copy_request_variable(void *pDest TSRMLS_DC, int num_args, va_list args, zend_hash_key *hash_key) { @@ -478,35 +479,6 @@ static int copy_request_variable(void *pDest TSRMLS_DC, int num_args, va_list ar return 0; } - if (Z_STRVAL(new_key)[0] == 'H') { - if ((strcmp(Z_STRVAL(new_key), "HTTP_GET_VARS")==0)|| - (strcmp(Z_STRVAL(new_key), "HTTP_POST_VARS")==0)|| - (strcmp(Z_STRVAL(new_key), "HTTP_POST_FILES")==0)|| - (strcmp(Z_STRVAL(new_key), "HTTP_ENV_VARS")==0)|| - (strcmp(Z_STRVAL(new_key), "HTTP_SERVER_VARS")==0)|| - (strcmp(Z_STRVAL(new_key), "HTTP_SESSION_VARS")==0)|| - (strcmp(Z_STRVAL(new_key), "HTTP_COOKIE_VARS")==0)|| - (strcmp(Z_STRVAL(new_key), "HTTP_RAW_POST_DATA")==0)) { - zval_dtor(&new_key); - return 0; - } - } else if (Z_STRVAL(new_key)[0] == '_') { - if ((strcmp(Z_STRVAL(new_key), "_COOKIE")==0)|| - (strcmp(Z_STRVAL(new_key), "_ENV")==0)|| - (strcmp(Z_STRVAL(new_key), "_FILES")==0)|| - (strcmp(Z_STRVAL(new_key), "_GET")==0)|| - (strcmp(Z_STRVAL(new_key), "_POST")==0)|| - (strcmp(Z_STRVAL(new_key), "_REQUEST")==0)|| - (strcmp(Z_STRVAL(new_key), "_SESSION")==0)|| - (strcmp(Z_STRVAL(new_key), "_SERVER")==0)) { - zval_dtor(&new_key); - return 0; - } - } else if (strcmp(Z_STRVAL(new_key), "GLOBALS")==0) { - zval_dtor(&new_key); - return 0; - } - zend_delete_global_variable(Z_STRVAL(new_key), Z_STRLEN(new_key) TSRMLS_CC); ZEND_SET_SYMBOL_WITH_LENGTH(&EG(symbol_table), Z_STRVAL(new_key), Z_STRLEN(new_key) + 1, *var, Z_REFCOUNT_PP(var) + 1, 0); @@ -554,35 +526,6 @@ static int copy_request_variable(void *pDest, int num_args, va_list args, zend_h return 0; } - if (new_key[0] == 'H') { - if ((strcmp(new_key, "HTTP_GET_VARS")==0)|| - (strcmp(new_key, "HTTP_POST_VARS")==0)|| - (strcmp(new_key, "HTTP_POST_FILES")==0)|| - (strcmp(new_key, "HTTP_ENV_VARS")==0)|| - (strcmp(new_key, "HTTP_SERVER_VARS")==0)|| - (strcmp(new_key, "HTTP_SESSION_VARS")==0)|| - (strcmp(new_key, "HTTP_COOKIE_VARS")==0)|| - (strcmp(new_key, "HTTP_RAW_POST_DATA")==0)) { - efree(new_key); - return 0; - } - } else if (new_key[0] == '_') { - if ((strcmp(new_key, "_COOKIE")==0)|| - (strcmp(new_key, "_ENV")==0)|| - (strcmp(new_key, "_FILES")==0)|| - (strcmp(new_key, "_GET")==0)|| - (strcmp(new_key, "_POST")==0)|| - (strcmp(new_key, "_REQUEST")==0)|| - (strcmp(new_key, "_SESSION")==0)|| - (strcmp(new_key, "_SERVER")==0)) { - efree(new_key); - return 0; - } - } else if (strcmp(new_key, "GLOBALS")==0) { - efree(new_key); - return 0; - } - #if PHP_MAJOR_VERSION > 5 || (PHP_MAJOR_VERSION == 5 && PHP_MINOR_VERSION > 0) zend_delete_global_variable(new_key, new_key_len-1 TSRMLS_CC); #else -- cgit v1.3 From 1ad8eaa898bfc78df3850ba5eb9a84a60ab56b70 Mon Sep 17 00:00:00 2001 From: Ben Fuhrmannek Date: Thu, 17 Jul 2014 13:41:32 +0200 Subject: import_request_variables() will only be replaced with PHP < 5.4.0 --- ex_imp.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'ex_imp.c') diff --git a/ex_imp.c b/ex_imp.c index 40d40e0..e1b93fc 100644 --- a/ex_imp.c +++ b/ex_imp.c @@ -444,6 +444,13 @@ PHP_FUNCTION(suhosin_extract) +#if PHP_VERSION_ID < 50400 +/* import_request_variables() has been DEPRECATED as of PHP 5.3.0 and REMOVED as of PHP 5.4.0. */ +#define SUHOSIN_HAVE_IRV 1 +#endif + +#ifdef SUHOSIN_HAVE_IRV + #if PHP_VERSION_ID >= 50300 static int copy_request_variable(void *pDest TSRMLS_DC, int num_args, va_list args, zend_hash_key *hash_key) { @@ -657,22 +664,28 @@ PHP_FUNCTION(suhosin_import_request_variables) } /* }}} */ +#endif /* SUHOSIN_HAVE_IRV */ + ZEND_BEGIN_ARG_INFO_EX(suhosin_arginfo_extract, 0, 0, 1) ZEND_ARG_INFO(ZEND_SEND_PREFER_REF, arg) /* ARRAY_INFO(0, arg, 0) */ ZEND_ARG_INFO(0, extract_type) ZEND_ARG_INFO(0, prefix) ZEND_END_ARG_INFO() +#ifdef SUHOSIN_HAVE_IRV ZEND_BEGIN_ARG_INFO_EX(suhosin_arginfo_import_request_variables, 0, 0, 1) ZEND_ARG_INFO(0, types) ZEND_ARG_INFO(0, prefix) ZEND_END_ARG_INFO() +#endif /* {{{ suhosin_ex_imp_functions[] */ zend_function_entry suhosin_ex_imp_functions[] = { PHP_NAMED_FE(extract, PHP_FN(suhosin_extract), suhosin_arginfo_extract) +#ifdef SUHOSIN_HAVE_IRV PHP_NAMED_FE(import_request_variables, PHP_FN(suhosin_import_request_variables), suhosin_arginfo_import_request_variables) +#endif {NULL, NULL, NULL} }; /* }}} */ @@ -683,7 +696,9 @@ void suhosin_hook_ex_imp() /* replace the extract and import_request_variables functions */ zend_hash_del(CG(function_table), "extract", sizeof("extract")); +#ifdef SUHOSIN_HAVE_IRV zend_hash_del(CG(function_table), "import_request_variables", sizeof("import_request_variables")); +#endif #ifndef ZEND_ENGINE_2 zend_register_functions(suhosin_ex_imp_functions, NULL, MODULE_PERSISTENT TSRMLS_CC); #else -- cgit v1.3 From 64d667b91a0f9c140383b8f74ea470fb8bd1da5e Mon Sep 17 00:00:00 2001 From: Ben Fuhrmannek Date: Thu, 17 Jul 2014 13:54:07 +0200 Subject: more redundancy removed / varname checks --- ex_imp.c | 24 +----------------------- 1 file changed, 1 insertion(+), 23 deletions(-) (limited to 'ex_imp.c') diff --git a/ex_imp.c b/ex_imp.c index e1b93fc..15bad3d 100644 --- a/ex_imp.c +++ b/ex_imp.c @@ -74,29 +74,7 @@ static int php_valid_var_name(char *var_name, int len) /* {{{ */ } } - if (var_name[0] == 'H') { - if ((strcmp(var_name, "HTTP_GET_VARS")==0)|| - (strcmp(var_name, "HTTP_POST_VARS")==0)|| - (strcmp(var_name, "HTTP_POST_FILES")==0)|| - (strcmp(var_name, "HTTP_ENV_VARS")==0)|| - (strcmp(var_name, "HTTP_SERVER_VARS")==0)|| - (strcmp(var_name, "HTTP_SESSION_VARS")==0)|| - (strcmp(var_name, "HTTP_COOKIE_VARS")==0)|| - (strcmp(var_name, "HTTP_RAW_POST_DATA")==0)) { - return 0; - } - } else if (var_name[0] == '_') { - if ((strcmp(var_name, "_COOKIE")==0)|| - (strcmp(var_name, "_ENV")==0)|| - (strcmp(var_name, "_FILES")==0)|| - (strcmp(var_name, "_GET")==0)|| - (strcmp(var_name, "_POST")==0)|| - (strcmp(var_name, "_REQUEST")==0)|| - (strcmp(var_name, "_SESSION")==0)|| - (strcmp(var_name, "_SERVER")==0)) { - return 0; - } - } else if (strcmp(var_name, "GLOBALS")==0) { + if (php_varname_check(var_name, len, 0 TSRMLS_CC) == FAILURE) { return 0; } -- cgit v1.3 From 02f527a6948558636ea04198a61e4a8d231f0484 Mon Sep 17 00:00:00 2001 From: Ben Fuhrmannek Date: Thu, 17 Jul 2014 13:59:29 +0200 Subject: some php_varname_check()s may be silent as we produce custom errors --- ex_imp.c | 2 +- ifilter.c | 2 +- ufilter.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) (limited to 'ex_imp.c') diff --git a/ex_imp.c b/ex_imp.c index 15bad3d..2ea2b99 100644 --- a/ex_imp.c +++ b/ex_imp.c @@ -74,7 +74,7 @@ static int php_valid_var_name(char *var_name, int len) /* {{{ */ } } - if (php_varname_check(var_name, len, 0 TSRMLS_CC) == FAILURE) { + if (php_varname_check(var_name, len, 1 TSRMLS_CC) == FAILURE) { return 0; } diff --git a/ifilter.c b/ifilter.c index 65b48cd..d85c3c2 100644 --- a/ifilter.c +++ b/ifilter.c @@ -620,7 +620,7 @@ unsigned int suhosin_input_filter(int arg, char *var, char **val, unsigned int v /* Drop this variable if it is one of GLOBALS, _GET, _POST, ... */ /* This is to protect several silly scripts that do globalizing themself */ - if (php_varname_check(var, var_len, 0 TSRMLS_CC) == FAILURE) { + if (php_varname_check(var, var_len, 1 TSRMLS_CC) == FAILURE) { suhosin_log(S_VARS, "tried to register forbidden variable '%s' through %s variables", var, arg == PARSE_GET ? "GET" : arg == PARSE_POST ? "POST" : "COOKIE"); if (!SUHOSIN_G(simulation)) { return 0; diff --git a/ufilter.c b/ufilter.c index 6775ec1..6464ce6 100644 --- a/ufilter.c +++ b/ufilter.c @@ -133,7 +133,7 @@ static int check_fileupload_varname(char *varname) /* Drop this variable if it is one of GLOBALS, _GET, _POST, ... */ /* This is to protect several silly scripts that do globalizing themself */ - if (php_varname_check(var, var_len, 0 TSRMLS_CC) == FAILURE) { + if (php_varname_check(var, var_len, 1 TSRMLS_CC) == FAILURE) { suhosin_log(S_FILES, "tried to register forbidden variable '%s' through FILE variables", var); if (!SUHOSIN_G(simulation)) { goto return_failure; -- cgit v1.3 From 3d5192e407f88d8a55822c081b22450016b70932 Mon Sep 17 00:00:00 2001 From: Ben Fuhrmannek Date: Wed, 23 Jul 2014 23:34:12 +0200 Subject: re-introduced suhosin_is_protected_varname as extra varname check --- ex_imp.c | 8 +++--- ifilter.c | 2 +- php_suhosin.h | 85 ++++++++++++++++++++++++++++++++++++++++++++++------------- ufilter.c | 2 +- 4 files changed, 73 insertions(+), 24 deletions(-) (limited to 'ex_imp.c') diff --git a/ex_imp.c b/ex_imp.c index 2ea2b99..6256f35 100644 --- a/ex_imp.c +++ b/ex_imp.c @@ -74,7 +74,7 @@ static int php_valid_var_name(char *var_name, int len) /* {{{ */ } } - if (php_varname_check(var_name, len, 1 TSRMLS_CC) == FAILURE) { + if (suhosin_is_protected_varname(var_name, len)) { return 0; } @@ -459,7 +459,7 @@ static int copy_request_variable(void *pDest TSRMLS_DC, int num_args, va_list ar zval_dtor(&num); } - if (php_varname_check(Z_STRVAL(new_key), Z_STRLEN(new_key), 0 TSRMLS_CC) == FAILURE) { + if (php_varname_check(Z_STRVAL(new_key), Z_STRLEN(new_key), 1 TSRMLS_CC) == FAILURE || suhosin_is_protected_varname(Z_STRVAL(new_key), Z_STRLEN(new_key))) { zval_dtor(&new_key); return 0; } @@ -506,8 +506,8 @@ static int copy_request_variable(void *pDest, int num_args, va_list args, zend_h new_key_len++; } - if (php_varname_check(new_key, new_key_len-1, 0 TSRMLS_CC) == FAILURE) { - zval_dtor(&new_key); + if (php_varname_check(new_key, new_key_len-1, 1 TSRMLS_CC) == FAILURE || suhosin_is_protected_varname(new_key, new_key_len-1)) { + efree(new_key); return 0; } diff --git a/ifilter.c b/ifilter.c index d85c3c2..b9da668 100644 --- a/ifilter.c +++ b/ifilter.c @@ -620,7 +620,7 @@ unsigned int suhosin_input_filter(int arg, char *var, char **val, unsigned int v /* Drop this variable if it is one of GLOBALS, _GET, _POST, ... */ /* This is to protect several silly scripts that do globalizing themself */ - if (php_varname_check(var, var_len, 1 TSRMLS_CC) == FAILURE) { + if (suhosin_is_protected_varname(var, var_len)) { suhosin_log(S_VARS, "tried to register forbidden variable '%s' through %s variables", var, arg == PARSE_GET ? "GET" : arg == PARSE_POST ? "POST" : "COOKIE"); if (!SUHOSIN_G(simulation)) { return 0; diff --git a/php_suhosin.h b/php_suhosin.h index e89d02b..b80d9b9 100644 --- a/php_suhosin.h +++ b/php_suhosin.h @@ -70,24 +70,74 @@ PHP_MINFO_FUNCTION(suhosin); #include "ext/standard/basic_functions.h" +static inline int suhosin_is_protected_varname(char *var, int var_len) +{ + switch (var_len) { + case 18: + if (memcmp(var, "HTTP_RAW_POST_DATA", 18)==0) goto protected_varname; + break; + case 17: + if (memcmp(var, "HTTP_SESSION_VARS", 17)==0) goto protected_varname; + break; + case 16: + if (memcmp(var, "HTTP_SERVER_VARS", 16)==0) goto protected_varname; + if (memcmp(var, "HTTP_COOKIE_VARS", 16)==0) goto protected_varname; + break; + case 15: + if (memcmp(var, "HTTP_POST_FILES", 15)==0) goto protected_varname; + break; + case 14: + if (memcmp(var, "HTTP_POST_VARS", 14)==0) goto protected_varname; + break; + case 13: + if (memcmp(var, "HTTP_GET_VARS", 13)==0) goto protected_varname; + if (memcmp(var, "HTTP_ENV_VARS", 13)==0) goto protected_varname; + break; + case 8: + if (memcmp(var, "_SESSION", 8)==0) goto protected_varname; + if (memcmp(var, "_REQUEST", 8)==0) goto protected_varname; + break; + case 7: + if (memcmp(var, "GLOBALS", 7)==0) goto protected_varname; + if (memcmp(var, "_COOKIE", 7)==0) goto protected_varname; + if (memcmp(var, "_SERVER", 7)==0) goto protected_varname; + break; + case 6: + if (memcmp(var, "_FILES", 6)==0) goto protected_varname; + break; + case 5: + if (memcmp(var, "_POST", 5)==0) goto protected_varname; + break; + case 4: + if (memcmp(var, "_ENV", 4)==0) goto protected_varname; + if (memcmp(var, "_GET", 4)==0) goto protected_varname; + break; + } + + return 0; +protected_varname: + return 1; +} + + #if PHP_VERSION_ID < 50203 static inline int php_varname_check(char *name, int name_len, zend_bool silent TSRMLS_DC) /* {{{ */ { - if (name_len == sizeof("GLOBALS") && !memcmp(name, "GLOBALS", sizeof("GLOBALS"))) { + if (name_len == sizeof("GLOBALS") - 1 && !memcmp(name, "GLOBALS", sizeof("GLOBALS") - 1)) { if (!silent) { php_error_docref(NULL TSRMLS_CC, E_WARNING, "Attempted GLOBALS variable overwrite"); } return FAILURE; } else if (name[0] == '_' && ( - (name_len == sizeof("_GET") && !memcmp(name, "_GET", sizeof("_GET"))) || - (name_len == sizeof("_POST") && !memcmp(name, "_POST", sizeof("_POST"))) || - (name_len == sizeof("_COOKIE") && !memcmp(name, "_COOKIE", sizeof("_COOKIE"))) || - (name_len == sizeof("_ENV") && !memcmp(name, "_ENV", sizeof("_ENV"))) || - (name_len == sizeof("_SERVER") && !memcmp(name, "_SERVER", sizeof("_SERVER"))) || - (name_len == sizeof("_SESSION") && !memcmp(name, "_SESSION", sizeof("_SESSION"))) || - (name_len == sizeof("_FILES") && !memcmp(name, "_FILES", sizeof("_FILES"))) || - (name_len == sizeof("_REQUEST") && !memcmp(name, "_REQUEST", sizeof("_REQUEST"))) + (name_len == sizeof("_GET") - 1 && !memcmp(name, "_GET", sizeof("_GET") - 1)) || + (name_len == sizeof("_POST") - 1 && !memcmp(name, "_POST", sizeof("_POST") - 1)) || + (name_len == sizeof("_COOKIE") - 1 && !memcmp(name, "_COOKIE", sizeof("_COOKIE") - 1)) || + (name_len == sizeof("_ENV") - 1 && !memcmp(name, "_ENV", sizeof("_ENV") - 1)) || + (name_len == sizeof("_SERVER") - 1 && !memcmp(name, "_SERVER", sizeof("_SERVER") - 1)) || + (name_len == sizeof("_SESSION") - 1 && !memcmp(name, "_SESSION", sizeof("_SESSION") - 1)) || + (name_len == sizeof("_FILES") - 1 && !memcmp(name, "_FILES", sizeof("_FILES") - 1)) || + (name_len == sizeof("_REQUEST") -1 && !memcmp(name, "_REQUEST", sizeof("_REQUEST") - 1)) ) ) { if (!silent) { @@ -96,14 +146,14 @@ static inline int php_varname_check(char *name, int name_len, zend_bool silent T return FAILURE; } else if (name[0] == 'H' && ( - (name_len == sizeof("HTTP_POST_VARS") && !memcmp(name, "HTTP_POST_VARS", sizeof("HTTP_POST_VARS"))) || - (name_len == sizeof("HTTP_GET_VARS") && !memcmp(name, "HTTP_GET_VARS", sizeof("HTTP_GET_VARS"))) || - (name_len == sizeof("HTTP_COOKIE_VARS") && !memcmp(name, "HTTP_COOKIE_VARS", sizeof("HTTP_COOKIE_VARS"))) || - (name_len == sizeof("HTTP_ENV_VARS") && !memcmp(name, "HTTP_ENV_VARS", sizeof("HTTP_ENV_VARS"))) || - (name_len == sizeof("HTTP_SERVER_VARS") && !memcmp(name, "HTTP_SERVER_VARS", sizeof("HTTP_SERVER_VARS"))) || - (name_len == sizeof("HTTP_SESSION_VARS") && !memcmp(name, "HTTP_SESSION_VARS", sizeof("HTTP_SESSION_VARS"))) || - (name_len == sizeof("HTTP_RAW_POST_DATA") && !memcmp(name, "HTTP_RAW_POST_DATA", sizeof("HTTP_RAW_POST_DATA"))) || - (name_len == sizeof("HTTP_POST_FILES") && !memcmp(name, "HTTP_POST_FILES", sizeof("HTTP_POST_FILES"))) + (name_len == sizeof("HTTP_POST_VARS") - 1 && !memcmp(name, "HTTP_POST_VARS", sizeof("HTTP_POST_VARS") - 1)) || + (name_len == sizeof("HTTP_GET_VARS") - 1 && !memcmp(name, "HTTP_GET_VARS", sizeof("HTTP_GET_VARS") - 1)) || + (name_len == sizeof("HTTP_COOKIE_VARS") - 1 && !memcmp(name, "HTTP_COOKIE_VARS", sizeof("HTTP_COOKIE_VARS") - 1)) || + (name_len == sizeof("HTTP_ENV_VARS") - 1 && !memcmp(name, "HTTP_ENV_VARS", sizeof("HTTP_ENV_VARS") - 1)) || + (name_len == sizeof("HTTP_SERVER_VARS") - 1 && !memcmp(name, "HTTP_SERVER_VARS", sizeof("HTTP_SERVER_VARS") - 1)) || + (name_len == sizeof("HTTP_SESSION_VARS") - 1 && !memcmp(name, "HTTP_SESSION_VARS", sizeof("HTTP_SESSION_VARS") - 1)) || + (name_len == sizeof("HTTP_RAW_POST_DATA") - 1 && !memcmp(name, "HTTP_RAW_POST_DATA", sizeof("HTTP_RAW_POST_DATA") - 1)) || + (name_len == sizeof("HTTP_POST_FILES") - 1 && !memcmp(name, "HTTP_POST_FILES", sizeof("HTTP_POST_FILES") - 1)) ) ) { if (!silent) { @@ -113,7 +163,6 @@ static inline int php_varname_check(char *name, int name_len, zend_bool silent T } return SUCCESS; } -/* }}} */ #endif ZEND_BEGIN_MODULE_GLOBALS(suhosin) diff --git a/ufilter.c b/ufilter.c index 6464ce6..10102aa 100644 --- a/ufilter.c +++ b/ufilter.c @@ -133,7 +133,7 @@ static int check_fileupload_varname(char *varname) /* Drop this variable if it is one of GLOBALS, _GET, _POST, ... */ /* This is to protect several silly scripts that do globalizing themself */ - if (php_varname_check(var, var_len, 1 TSRMLS_CC) == FAILURE) { + if (php_varname_check(var, var_len, 1 TSRMLS_CC) == FAILURE || suhosin_is_protected_varname(var, var_len)) { suhosin_log(S_FILES, "tried to register forbidden variable '%s' through FILE variables", var); if (!SUHOSIN_G(simulation)) { goto return_failure; -- cgit v1.3