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 --- ufilter.c | 52 ++-------------------------------------------------- 1 file changed, 2 insertions(+), 50 deletions(-) (limited to 'ufilter.c') diff --git a/ufilter.c b/ufilter.c index 6d9669f..6775ec1 100644 --- a/ufilter.c +++ b/ufilter.c @@ -30,60 +30,13 @@ #include "php_suhosin.h" #include "php_variables.h" #include "suhosin_rfc1867.h" +#include "ext/standard/php_var.h" PHP_SUHOSIN_API int (*old_rfc1867_callback)(unsigned int event, void *event_data, void **extra TSRMLS_DC) = NULL; #if !HAVE_RFC1867_CALLBACK PHP_SUHOSIN_API int (*php_rfc1867_callback)(unsigned int event, void *event_data, void **extra TSRMLS_DC) = NULL; #endif -static int is_protected_varname(char *var, int var_len) -{ - switch (var_len) { - case 18: - if (memcmp(var, "HTTP_RAW_POST_DATA", 18)==0) goto protected_varname2; - break; - case 17: - if (memcmp(var, "HTTP_SESSION_VARS", 17)==0) goto protected_varname2; - break; - case 16: - if (memcmp(var, "HTTP_SERVER_VARS", 16)==0) goto protected_varname2; - if (memcmp(var, "HTTP_COOKIE_VARS", 16)==0) goto protected_varname2; - break; - case 15: - if (memcmp(var, "HTTP_POST_FILES", 15)==0) goto protected_varname2; - break; - case 14: - if (memcmp(var, "HTTP_POST_VARS", 14)==0) goto protected_varname2; - break; - case 13: - if (memcmp(var, "HTTP_GET_VARS", 13)==0) goto protected_varname2; - if (memcmp(var, "HTTP_ENV_VARS", 13)==0) goto protected_varname2; - break; - case 8: - if (memcmp(var, "_SESSION", 8)==0) goto protected_varname2; - if (memcmp(var, "_REQUEST", 8)==0) goto protected_varname2; - break; - case 7: - if (memcmp(var, "GLOBALS", 7)==0) goto protected_varname2; - if (memcmp(var, "_COOKIE", 7)==0) goto protected_varname2; - if (memcmp(var, "_SERVER", 7)==0) goto protected_varname2; - break; - case 6: - if (memcmp(var, "_FILES", 6)==0) goto protected_varname2; - break; - case 5: - if (memcmp(var, "_POST", 5)==0) goto protected_varname2; - break; - case 4: - if (memcmp(var, "_ENV", 4)==0) goto protected_varname2; - if (memcmp(var, "_GET", 4)==0) goto protected_varname2; - break; - } - - return 0; -protected_varname2: - return 1; -} /* {{{ SAPI_UPLOAD_VARNAME_FILTER_FUNC */ @@ -180,8 +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 (is_protected_varname(var, var_len)) { + if (php_varname_check(var, var_len, 0 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 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 'ufilter.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 'ufilter.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