From bab54996ce99bf66bc3e0e377fecb37e52894dd7 Mon Sep 17 00:00:00 2001 From: Ben Fuhrmannek Date: Tue, 15 Jul 2014 15:00:19 +0200 Subject: simplified else/break --- execute.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/execute.c b/execute.c index 82a4866..9bcd29c 100644 --- a/execute.c +++ b/execute.c @@ -632,7 +632,6 @@ not_evaled_code: case SUHOSIN_CODE_TYPE_UNKNOWN: case SUHOSIN_CODE_TYPE_GOODFILE: goto continue_execution; - break; } continue_execution: @@ -1060,9 +1059,8 @@ int ih_fixusername(IH_HANDLER_PARAMS) if (!SUHOSIN_G(simulation)) { RETVAL_FALSE; return (1); - } else { - break; } + break; } cp++; } -- cgit v1.3 From 9cb8c448eee428ee1d64323e3e71d4a54a25154a Mon Sep 17 00:00:00 2001 From: Ben Fuhrmannek Date: Tue, 15 Jul 2014 15:43:14 +0200 Subject: replaced suhosin_register_cookie_variable + suhosin_register_cookie_variable_safe --- suhosin.c | 179 +++----------------------------------------------------------- 1 file changed, 6 insertions(+), 173 deletions(-) diff --git a/suhosin.c b/suhosin.c index f57e85a..7fbefc8 100644 --- a/suhosin.c +++ b/suhosin.c @@ -28,6 +28,7 @@ #include "zend_extensions.h" #include "ext/standard/info.h" #include "php_syslog.h" +#include "php_variables.h" #include "php_suhosin.h" #include "zend_llist.h" #include "zend_operators.h" @@ -618,175 +619,6 @@ static ZEND_INI_MH(OnUpdate_fail) return FAILURE; } -/* {{{ proto void suhosin_register_cookie_variable(char *var, zval *val, zval *track_vars_array TSRMLS_DC) - Registers a cookie in the RAW cookie array */ -static void suhosin_register_cookie_variable(char *var, zval *val, zval *track_vars_array TSRMLS_DC) -{ - char *p = NULL; - char *ip; /* index pointer */ - char *index, *escaped_index = NULL; - int var_len, index_len; - zval *gpc_element, **gpc_element_p; - zend_bool is_array = 0; - HashTable *symtable1 = NULL; - - assert(var != NULL); - - symtable1 = Z_ARRVAL_P(track_vars_array); - - /* - * Prepare variable name - */ - - /* ignore leading spaces in the variable name */ - while (*var && *var==' ') { - var++; - } - - /* ensure that we don't have spaces or dots in the variable name (not binary safe) */ - for (p = var; *p; p++) { - if (*p == ' ' || *p == '.') { - *p='_'; - } else if (*p == '[') { - is_array = 1; - ip = p; - *p = 0; - break; - } - } - var_len = p - var; - - if (var_len==0) { /* empty variable name, or variable name with a space in it */ - zval_dtor(val); - return; - } - - index = var; - index_len = var_len; - - if (is_array) { - while (1) { - char *index_s; - int new_idx_len = 0; - - ip++; - index_s = ip; - if (isspace(*ip)) { - ip++; - } - if (*ip==']') { - index_s = NULL; - } else { - ip = strchr(ip, ']'); - if (!ip) { - /* PHP variables cannot contain '[' in their names, so we replace the character with a '_' */ - *(index_s - 1) = '_'; - - index_len = var_len = 0; - if (index) { - index_len = var_len = strlen(index); - } - goto plain_var; - return; - } - *ip = 0; - new_idx_len = strlen(index_s); - } - - if (!index) { - MAKE_STD_ZVAL(gpc_element); - array_init(gpc_element); - zend_hash_next_index_insert(symtable1, &gpc_element, sizeof(zval *), (void **) &gpc_element_p); - } else { -#if PHP_VERSION_ID < 50400 - if (PG(magic_quotes_gpc) && (index != var)) { - /* no need to addslashes() the index if it's the main variable name */ - escaped_index = php_addslashes(index, index_len, &index_len, 0 TSRMLS_CC); - } else { -#endif - escaped_index = index; -#if PHP_VERSION_ID < 50400 - } -#endif - if (zend_symtable_find(symtable1, escaped_index, index_len + 1, (void **) &gpc_element_p) == FAILURE - || Z_TYPE_PP(gpc_element_p) != IS_ARRAY) { - MAKE_STD_ZVAL(gpc_element); - array_init(gpc_element); - zend_symtable_update(symtable1, escaped_index, index_len + 1, &gpc_element, sizeof(zval *), (void **) &gpc_element_p); - } - if (index != escaped_index) { - efree(escaped_index); - } - } - symtable1 = Z_ARRVAL_PP(gpc_element_p); - /* ip pointed to the '[' character, now obtain the key */ - index = index_s; - index_len = new_idx_len; - - ip++; - if (*ip == '[') { - is_array = 1; - *ip = 0; - } else { - goto plain_var; - } - } - } else { -plain_var: - MAKE_STD_ZVAL(gpc_element); - gpc_element->value = val->value; - Z_TYPE_P(gpc_element) = Z_TYPE_P(val); - if (!index) { - zend_hash_next_index_insert(symtable1, &gpc_element, sizeof(zval *), (void **) &gpc_element_p); - } else { -#if PHP_VERSION_ID < 50400 - if (PG(magic_quotes_gpc)) { - escaped_index = php_addslashes(index, index_len, &index_len, 0 TSRMLS_CC); - } else { -#endif - escaped_index = index; -#if PHP_VERSION_ID < 50400 - } -#endif - /* - * According to rfc2965, more specific paths are listed above the less specific ones. - * If we encounter a duplicate cookie name, we should skip it, since it is not possible - * to have the same (plain text) cookie name for the same path and we should not overwrite - * more specific cookies with the less specific ones. - */ - if (zend_symtable_exists(symtable1, escaped_index, index_len + 1)) { - zval_ptr_dtor(&gpc_element); - } else { - zend_symtable_update(symtable1, escaped_index, index_len + 1, &gpc_element, sizeof(zval *), (void **) &gpc_element_p); - } - if (escaped_index != index) { - efree(escaped_index); - } - } - } -} -/* }}} */ - -static void suhosin_register_cookie_variable_safe(char *var, char *strval, int str_len, zval *track_vars_array TSRMLS_DC) -{ - zval new_entry; - assert(strval != NULL); - - /* Prepare value */ - Z_STRLEN(new_entry) = str_len; -#if PHP_VERSION_ID < 50400 - if (PG(magic_quotes_gpc)) { - Z_STRVAL(new_entry) = php_addslashes(strval, Z_STRLEN(new_entry), &Z_STRLEN(new_entry), 0 TSRMLS_CC); - } else { -#endif - Z_STRVAL(new_entry) = estrndup(strval, Z_STRLEN(new_entry)); -#if PHP_VERSION_ID < 50400 - } -#endif - Z_TYPE(new_entry) = IS_STRING; - - suhosin_register_cookie_variable(var, &new_entry, track_vars_array TSRMLS_CC); -} /* {{{ proto string suhosin_encrypt_cookie(string name, string value) @@ -833,7 +665,7 @@ static PHP_FUNCTION(suhosin_get_raw_cookies) int val_len; array_init(array_ptr); - + SDEBUG("get_raw_cookies %s", SUHOSIN_G(raw_cookie)); if (SUHOSIN_G(raw_cookie)) { res = estrdup(SUHOSIN_G(raw_cookie)); } else { @@ -843,17 +675,18 @@ static PHP_FUNCTION(suhosin_get_raw_cookies) var = php_strtok_r(res, ";", &strtok_buf); while (var) { + SDEBUG("raw cookie: %s", var); val = strchr(var, '='); if (val) { /* have a value */ *val++ = '\0'; php_url_decode(var, strlen(var)); val_len = php_url_decode(val, strlen(val)); - suhosin_register_cookie_variable_safe(var, val, val_len, array_ptr TSRMLS_CC); + php_register_variable_safe(var, val, val_len, array_ptr TSRMLS_CC); } else { php_url_decode(var, strlen(var)); val_len = 0; val = ""; - suhosin_register_cookie_variable_safe(var, "", 0, array_ptr TSRMLS_CC); + php_register_variable_safe(var, "", 0, array_ptr TSRMLS_CC); } var = php_strtok_r(NULL, ";", &strtok_buf); } @@ -1049,7 +882,7 @@ char *suhosin_getenv(char *name, size_t name_len TSRMLS_DC) tmp = getenv(name); efree(name); if (tmp) { - return(estrdup(tmp)); + return estrdup(tmp); } } return NULL; -- cgit v1.3 From f9bb4240aee34029450951394e4d0474f34a3a51 Mon Sep 17 00:00:00 2001 From: Ben Fuhrmannek Date: Wed, 16 Jul 2014 12:43:09 +0200 Subject: updated suhosin version --- php_suhosin.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/php_suhosin.h b/php_suhosin.h index 4b460e4..22e6df1 100644 --- a/php_suhosin.h +++ b/php_suhosin.h @@ -22,7 +22,7 @@ #ifndef PHP_SUHOSIN_H #define PHP_SUHOSIN_H -#define SUHOSIN_EXT_VERSION "0.9.36" +#define SUHOSIN_EXT_VERSION "0.9.37-dev" /*#define SUHOSIN_DEBUG*/ #define SUHOSIN_LOG "/tmp/suhosin_log.txt" -- cgit v1.3 From ace8fdae3788ca4381a17a14bc4d5acd0cd98709 Mon Sep 17 00:00:00 2001 From: Ben Fuhrmannek Date: Wed, 16 Jul 2014 13:21:21 +0200 Subject: rewrite of register_server_variables - less redundancy (may be slower though) --- ifilter.c | 72 +++++++++++++++++++++++++++------------------------------------ 1 file changed, 31 insertions(+), 41 deletions(-) diff --git a/ifilter.c b/ifilter.c index 8b2e8a3..7ac4637 100644 --- a/ifilter.c +++ b/ifilter.c @@ -150,7 +150,6 @@ static void suhosin_server_encode(HashTable *arr, char *key, int klen) temp = (unsigned char *)Z_STRVAL_PP(tzval); - t = temp; for (t = temp; *t; t++) { if (suhosin_is_dangerous_char[*t]) { extra += 2; @@ -186,54 +185,45 @@ static void suhosin_server_encode(HashTable *arr, char *key, int klen) */ void suhosin_register_server_variables(zval *track_vars_array TSRMLS_DC) { - HashTable *svars; - int retval, failure=0; - - orig_register_server_variables(track_vars_array TSRMLS_CC); + HashTable *svars; + int retval, failure=0, i; - svars = Z_ARRVAL_P(track_vars_array); - + char *varnames[] = { + "HTTP_GET_VARS", "HTTP_POST_VARS", "HTTP_COOKIE_VARS", + "HTTP_ENV_VARS", "HTTP_SERVER_VARS", "HTTP_SESSION_VARS", + "HTTP_POST_FILES", "HTTP_RAW_POST_DATA", + NULL + }; + + orig_register_server_variables(track_vars_array TSRMLS_CC); + + svars = Z_ARRVAL_P(track_vars_array); if (!SUHOSIN_G(simulation)) { - retval = zend_hash_del(svars, "HTTP_GET_VARS", sizeof("HTTP_GET_VARS")); - if (retval == SUCCESS) failure = 1; - retval = zend_hash_del(svars, "HTTP_POST_VARS", sizeof("HTTP_POST_VARS")); - if (retval == SUCCESS) failure = 1; - retval = zend_hash_del(svars, "HTTP_COOKIE_VARS", sizeof("HTTP_COOKIE_VARS")); - if (retval == SUCCESS) failure = 1; - retval = zend_hash_del(svars, "HTTP_ENV_VARS", sizeof("HTTP_ENV_VARS")); - if (retval == SUCCESS) failure = 1; - retval = zend_hash_del(svars, "HTTP_SERVER_VARS", sizeof("HTTP_SERVER_VARS")); - if (retval == SUCCESS) failure = 1; - retval = zend_hash_del(svars, "HTTP_SESSION_VARS", sizeof("HTTP_SESSION_VARS")); - if (retval == SUCCESS) failure = 1; - retval = zend_hash_del(svars, "HTTP_POST_FILES", sizeof("HTTP_POST_FILES")); - if (retval == SUCCESS) failure = 1; - retval = zend_hash_del(svars, "HTTP_RAW_POST_DATA", sizeof("HTTP_RAW_POST_DATA")); - if (retval == SUCCESS) failure = 1; + for (i = 0; varnames[i]; i++) { + retval = zend_hash_del(svars, varnames[i], strlen(varnames[i])+1); + if (retval == SUCCESS) failure = 1; + } } else { - retval = zend_hash_exists(svars, "HTTP_GET_VARS", sizeof("HTTP_GET_VARS")); - retval+= zend_hash_exists(svars, "HTTP_POST_VARS", sizeof("HTTP_POST_VARS")); - retval+= zend_hash_exists(svars, "HTTP_COOKIE_VARS", sizeof("HTTP_COOKIE_VARS")); - retval+= zend_hash_exists(svars, "HTTP_ENV_VARS", sizeof("HTTP_ENV_VARS")); - retval+= zend_hash_exists(svars, "HTTP_SERVER_VARS", sizeof("HTTP_SERVER_VARS")); - retval+= zend_hash_exists(svars, "HTTP_SESSION_VARS", sizeof("HTTP_SESSION_VARS")); - retval+= zend_hash_exists(svars, "HTTP_POST_FILES", sizeof("HTTP_POST_FILES")); - retval+= zend_hash_exists(svars, "HTTP_RAW_POST_DATA", sizeof("HTTP_RAW_POST_DATA")); - if (retval > 0) failure = 1; + for (i = 0; varnames[i]; i++) { + if (zend_hash_exists(svars, varnames[i], strlen(varnames[i])+1)) { + failure = 1; + break; + } + } + } + + if (failure) { + suhosin_log(S_VARS, "Attacker tried to overwrite a superglobal through a HTTP header"); } - - if (failure) { - suhosin_log(S_VARS, "Attacker tried to overwrite a superglobal through a HTTP header"); - } if (SUHOSIN_G(raw_cookie)) { - zval *z; - MAKE_STD_ZVAL(z); + zval *z; + MAKE_STD_ZVAL(z); ZVAL_STRING(z, SUHOSIN_G(raw_cookie), 1); zend_hash_add(svars, "RAW_HTTP_COOKIE", sizeof("RAW_HTTP_COOKIE"), (void **)&z, sizeof(zval *), NULL); - } - if (SUHOSIN_G(decrypted_cookie)) { - zval *z; + } + if (SUHOSIN_G(decrypted_cookie)) { + zval *z; MAKE_STD_ZVAL(z); ZVAL_STRING(z, SUHOSIN_G(decrypted_cookie), 0); zend_hash_update(svars, "HTTP_COOKIE", sizeof("HTTP_COOKIE"), (void **)&z, sizeof(zval *), NULL); -- cgit v1.3 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 +---------------------------------------------------------- ifilter.c | 54 ++++++------------------------------------------------ php_suhosin.h | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ ufilter.c | 52 ++-------------------------------------------------- 4 files changed, 59 insertions(+), 156 deletions(-) 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 diff --git a/ifilter.c b/ifilter.c index 7ac4637..65b48cd 100644 --- a/ifilter.c +++ b/ifilter.c @@ -29,6 +29,7 @@ #include "ext/standard/info.h" #include "php_suhosin.h" #include "php_variables.h" +#include "ext/standard/php_var.h" static void (*orig_register_server_variables)(zval *track_vars_array TSRMLS_DC) = NULL; @@ -619,47 +620,11 @@ 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 */ - - 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; + if (php_varname_check(var, var_len, 0 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; + } } /* Okay let PHP register this variable */ @@ -681,13 +646,6 @@ unsigned int suhosin_input_filter(int arg, char *var, char **val, unsigned int v } return 1; -protected_varname: - 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; - } else { - return 1; - } } /* }}} */ diff --git a/php_suhosin.h b/php_suhosin.h index 22e6df1..e89d02b 100644 --- a/php_suhosin.h +++ b/php_suhosin.h @@ -39,6 +39,10 @@ #endif #endif +#ifndef PHP_VERSION_ID +#define PHP_VERSION_ID (PHP_MAJOR_VERSION * 10000 + PHP_MINOR_VERSION * 100 + PHP_RELEASE_VERSION) +#endif + extern zend_module_entry suhosin_module_entry; #define phpext_suhosin_ptr &suhosin_module_entry @@ -66,6 +70,52 @@ PHP_MINFO_FUNCTION(suhosin); #include "ext/standard/basic_functions.h" +#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 (!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"))) + ) + ) { + if (!silent) { + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Attempted super-global (%s) variable overwrite", name); + } + 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"))) + ) + ) { + if (!silent) { + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Attempted long input array (%s) overwrite", name); + } + return FAILURE; + } + return SUCCESS; +} +/* }}} */ +#endif + ZEND_BEGIN_MODULE_GLOBALS(suhosin) zend_uint in_code_type; long execution_depth; 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 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(+) 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(-) 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(-) 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 fd00e0e1eedce6882632332774ab4fb278c2d5d3 Mon Sep 17 00:00:00 2001 From: Ben Fuhrmannek Date: Thu, 17 Jul 2014 14:12:25 +0200 Subject: minor changes / no more compiler warnings --- execute.c | 4 ++-- log.c | 4 ++-- session.c | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/execute.c b/execute.c index 9bcd29c..560d8f5 100644 --- a/execute.c +++ b/execute.c @@ -150,7 +150,7 @@ static int suhosin_check_filename(char *s, int len TSRMLS_DC) return SUHOSIN_CODE_TYPE_MANYDOTS; } -SDEBUG("xxx %08x %08x",SUHOSIN_G(include_whitelist),SUHOSIN_G(include_blacklist)); +SDEBUG("xxx %p %p",SUHOSIN_G(include_whitelist),SUHOSIN_G(include_blacklist)); /* no black or whitelist then disallow all */ if (SUHOSIN_G(include_whitelist)==NULL && SUHOSIN_G(include_blacklist)==NULL) { /* disallow all URLs */ @@ -519,7 +519,7 @@ static void suhosin_execute_ex(zend_op_array *op_array, int zo, long dummy TSRML } else { if (suhosin_zend_extension_entry.resource_number != -1) { suhosin_flags = (unsigned long *) &op_array->reserved[suhosin_zend_extension_entry.resource_number]; - SDEBUG("suhosin flags: %08x", *suhosin_flags); + SDEBUG("suhosin flags: %08lx", *suhosin_flags); if (*suhosin_flags & SUHOSIN_FLAG_CREATED_BY_EVAL) { SUHOSIN_G(in_code_type) = SUHOSIN_EVAL; diff --git a/log.c b/log.c index 3edc119..6f4a3eb 100644 --- a/log.c +++ b/log.c @@ -122,7 +122,7 @@ PHP_SUHOSIN_API void suhosin_log(int loglevel, char *fmt, ...) /* remove the S_GETCALLER flag */ loglevel = loglevel & ~S_GETCALLER; - SDEBUG("(suhosin_log) loglevel: %d log_syslog: %u - log_sapi: %u - log_script: %u", loglevel, SUHOSIN_G(log_syslog), SUHOSIN_G(log_sapi), SUHOSIN_G(log_script)); + SDEBUG("(suhosin_log) loglevel: %d log_syslog: %ld - log_sapi: %ld - log_script: %ld", loglevel, SUHOSIN_G(log_syslog), SUHOSIN_G(log_sapi), SUHOSIN_G(log_script)); /* dump core if wanted */ if (SUHOSIN_G(coredump) && loglevel == S_MEMORY) { @@ -278,7 +278,7 @@ log_file: log_sapi: /* SAPI Logging activated? */ - SDEBUG("(suhosin_log) log_syslog: %u - log_sapi: %u - log_script: %u - log_phpscript: %u", SUHOSIN_G(log_syslog), SUHOSIN_G(log_sapi), SUHOSIN_G(log_script), SUHOSIN_G(log_phpscript)); + SDEBUG("(suhosin_log) log_syslog: %ld - log_sapi: %ld - log_script: %ld - log_phpscript: %ld", SUHOSIN_G(log_syslog), SUHOSIN_G(log_sapi), SUHOSIN_G(log_script), SUHOSIN_G(log_phpscript)); if (((SUHOSIN_G(log_sapi)|S_INTERNAL) & loglevel)!=0) { #if PHP_VERSION_ID < 50400 sapi_module.log_message(buf); diff --git a/session.c b/session.c index 924469b..a3261c9 100644 --- a/session.c +++ b/session.c @@ -1020,7 +1020,7 @@ static PHP_INI_MH(suhosin_OnUpdateSaveHandler) int r; char *tmp; - if ((ps_mod_user) && (SUHOSIN_G(s_original_mod) == ps_mod_user) && (strcmp(new_value, "user") == NULL)) { + if ((ps_mod_user) && (SUHOSIN_G(s_original_mod) == ps_mod_user) && (strcmp(new_value, "user") == 0)) { return SUCCESS; } -- cgit v1.3 From 238f060a1362b9c6bf93aca2d45da6c2985fc3ca Mon Sep 17 00:00:00 2001 From: Ben Fuhrmannek Date: Thu, 17 Jul 2014 16:45:02 +0200 Subject: suhosin_get_raw_cookies() parses cookies in reverse order to give first occurrence precedence --- suhosin.c | 43 ++++++++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/suhosin.c b/suhosin.c index 7fbefc8..f5dde65 100644 --- a/suhosin.c +++ b/suhosin.c @@ -660,38 +660,43 @@ return_plain: static PHP_FUNCTION(suhosin_get_raw_cookies) { char *var, *val, *res; - zval *array_ptr = return_value; - char *strtok_buf = NULL; - int val_len; - + zval *array_ptr = return_value; + char *strtok_buf = NULL; + int val_len; + array_init(array_ptr); - SDEBUG("get_raw_cookies %s", SUHOSIN_G(raw_cookie)); - if (SUHOSIN_G(raw_cookie)) { - res = estrdup(SUHOSIN_G(raw_cookie)); - } else { - return; - } - - var = php_strtok_r(res, ";", &strtok_buf); - while (var) { - SDEBUG("raw cookie: %s", var); + if (SUHOSIN_G(raw_cookie)) { + res = estrdup(SUHOSIN_G(raw_cookie)); + } else { + return; + } + + var = NULL; + while (var != res) { + var = strrchr(res, ';'); + if (var) { + *var++ = '\0'; + } else { + var = res; + } + if (!*var) { continue; } + val = strchr(var, '='); if (val) { /* have a value */ *val++ = '\0'; php_url_decode(var, strlen(var)); val_len = php_url_decode(val, strlen(val)); - php_register_variable_safe(var, val, val_len, array_ptr TSRMLS_CC); } else { php_url_decode(var, strlen(var)); val_len = 0; val = ""; - php_register_variable_safe(var, "", 0, array_ptr TSRMLS_CC); } - var = php_strtok_r(NULL, ";", &strtok_buf); + php_register_variable_safe(var, val, val_len, array_ptr TSRMLS_CC); + } - - efree(res); + + efree(res); } /* }}} */ -- 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(-) 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 From 650d6c0c96e2a05b0deeca06a7104116d66b6894 Mon Sep 17 00:00:00 2001 From: Ben Fuhrmannek Date: Thu, 24 Jul 2014 00:04:34 +0200 Subject: reintroduced loop-free check for invalid varnames in suhosin_register_server_variables --- ifilter.c | 44 ++++++++++++++++++++++++++------------------ 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/ifilter.c b/ifilter.c index b9da668..dfe7c6b 100644 --- a/ifilter.c +++ b/ifilter.c @@ -187,30 +187,38 @@ static void suhosin_server_encode(HashTable *arr, char *key, int klen) void suhosin_register_server_variables(zval *track_vars_array TSRMLS_DC) { HashTable *svars; - int retval, failure=0, i; - - char *varnames[] = { - "HTTP_GET_VARS", "HTTP_POST_VARS", "HTTP_COOKIE_VARS", - "HTTP_ENV_VARS", "HTTP_SERVER_VARS", "HTTP_SESSION_VARS", - "HTTP_POST_FILES", "HTTP_RAW_POST_DATA", - NULL - }; + int retval = 0, failure = 0; orig_register_server_variables(track_vars_array TSRMLS_CC); svars = Z_ARRVAL_P(track_vars_array); if (!SUHOSIN_G(simulation)) { - for (i = 0; varnames[i]; i++) { - retval = zend_hash_del(svars, varnames[i], strlen(varnames[i])+1); - if (retval == SUCCESS) failure = 1; - } + retval = zend_hash_del(svars, "HTTP_GET_VARS", sizeof("HTTP_GET_VARS")); + if (retval == SUCCESS) failure = 1; + retval = zend_hash_del(svars, "HTTP_POST_VARS", sizeof("HTTP_POST_VARS")); + if (retval == SUCCESS) failure = 1; + retval = zend_hash_del(svars, "HTTP_COOKIE_VARS", sizeof("HTTP_COOKIE_VARS")); + if (retval == SUCCESS) failure = 1; + retval = zend_hash_del(svars, "HTTP_ENV_VARS", sizeof("HTTP_ENV_VARS")); + if (retval == SUCCESS) failure = 1; + retval = zend_hash_del(svars, "HTTP_SERVER_VARS", sizeof("HTTP_SERVER_VARS")); + if (retval == SUCCESS) failure = 1; + retval = zend_hash_del(svars, "HTTP_SESSION_VARS", sizeof("HTTP_SESSION_VARS")); + if (retval == SUCCESS) failure = 1; + retval = zend_hash_del(svars, "HTTP_POST_FILES", sizeof("HTTP_POST_FILES")); + if (retval == SUCCESS) failure = 1; + retval = zend_hash_del(svars, "HTTP_RAW_POST_DATA", sizeof("HTTP_RAW_POST_DATA")); + if (retval == SUCCESS) failure = 1; } else { - for (i = 0; varnames[i]; i++) { - if (zend_hash_exists(svars, varnames[i], strlen(varnames[i])+1)) { - failure = 1; - break; - } - } + retval = zend_hash_exists(svars, "HTTP_GET_VARS", sizeof("HTTP_GET_VARS")); + retval+= zend_hash_exists(svars, "HTTP_POST_VARS", sizeof("HTTP_POST_VARS")); + retval+= zend_hash_exists(svars, "HTTP_COOKIE_VARS", sizeof("HTTP_COOKIE_VARS")); + retval+= zend_hash_exists(svars, "HTTP_ENV_VARS", sizeof("HTTP_ENV_VARS")); + retval+= zend_hash_exists(svars, "HTTP_SERVER_VARS", sizeof("HTTP_SERVER_VARS")); + retval+= zend_hash_exists(svars, "HTTP_SESSION_VARS", sizeof("HTTP_SESSION_VARS")); + retval+= zend_hash_exists(svars, "HTTP_POST_FILES", sizeof("HTTP_POST_FILES")); + retval+= zend_hash_exists(svars, "HTTP_RAW_POST_DATA", sizeof("HTTP_RAW_POST_DATA")); + if (retval > 0) failure = 1; } if (failure) { -- cgit v1.3 From b074d630123a28cbca3babc756cbec6dede996f7 Mon Sep 17 00:00:00 2001 From: Ben Fuhrmannek Date: Fri, 1 Aug 2014 18:12:11 +0200 Subject: fixed potential segfault/hashtable inconsistency for disable_display_errors=fail --- suhosin.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/suhosin.c b/suhosin.c index f5dde65..7bd8902 100644 --- a/suhosin.c +++ b/suhosin.c @@ -982,11 +982,12 @@ PHP_MINIT_FUNCTION(suhosin) zend_ini_entry *i; if (zend_hash_find(EG(ini_directives), "display_errors", sizeof("display_errors"), (void **) &i) == SUCCESS) { if (i->on_modify) { + i->on_modify(i, "0", strlen("0"), i->mh_arg1, i->mh_arg2, i->mh_arg3, ZEND_INI_STAGE_STARTUP TSRMLS_CC); if (SUHOSIN_G(disable_display_errors) > 1) { - zend_alter_ini_entry_ex("display_errors", sizeof("display_errors"), "0", sizeof("0"), ZEND_INI_SYSTEM, ZEND_INI_STAGE_STARTUP, 0 TSRMLS_CC); + i->value = estrdup("0"); + i->value_length = strlen(i->value); i->on_modify = OnUpdate_fail; } else { - i->on_modify(i, "Off", sizeof("off"), i->mh_arg1, i->mh_arg2, i->mh_arg3, ZEND_INI_STAGE_STARTUP TSRMLS_CC); i->on_modify = NULL; } } -- cgit v1.3