From 3b6c6af3faa6a66e4f5337a769baed32f404b82b Mon Sep 17 00:00:00 2001 From: Stefan Esser Date: Sat, 14 Jan 2012 19:32:14 +0100 Subject: Use new suhosin_getenv() function in all places Add protection against mbstring Add detection of incompatible extensions that change POST handlers --- Changelog | 6 +++++- log.c | 6 +++--- php_suhosin.h | 1 + post_handler.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++ session.c | 8 ++++---- suhosin.c | 28 ++++++++++++++++++++++++++++ 6 files changed, 94 insertions(+), 8 deletions(-) diff --git a/Changelog b/Changelog index 8c2a8a4..c40b055 100644 --- a/Changelog +++ b/Changelog @@ -1,5 +1,9 @@ -2012-01-11 - 0.9.33-dev +2012-01-14 - 0.9.33-dev + - Make clear that suhosin is incompatible to mbstring.encoding_translation=On + - Stop mbstring extension from replacing POST handlers + - Added detection of extensions manipulating POST handlers + - Fixed environment variables for logging do not go through the filter extension anymore - Fixed stack based buffer overflow in transparent cookie encryption (see separate advisory) - Fixed that disabling HTTP response splitting protection also disabled NUL byte protection in HTTP headers - Removed crypt() support - because not used for PHP >= 5.3.0 anyway diff --git a/log.c b/log.c index cc297d2..2e3d7a1 100644 --- a/log.c +++ b/log.c @@ -118,12 +118,12 @@ PHP_SUHOSIN_API void suhosin_log(int loglevel, char *fmt, ...) } if (SUHOSIN_G(log_use_x_forwarded_for)) { - ip_address = sapi_getenv("HTTP_X_FORWARDED_FOR", 20 TSRMLS_CC); + ip_address = suhosin_getenv("HTTP_X_FORWARDED_FOR", 20 TSRMLS_CC); if (ip_address == NULL) { ip_address = "X-FORWARDED-FOR not set"; } } else { - ip_address = sapi_getenv("REMOTE_ADDR", 11 TSRMLS_CC); + ip_address = suhosin_getenv("REMOTE_ADDR", 11 TSRMLS_CC); if (ip_address == NULL) { ip_address = "REMOTE_ADDR not set"; } @@ -154,7 +154,7 @@ PHP_SUHOSIN_API void suhosin_log(int loglevel, char *fmt, ...) } ap_php_snprintf(buf, sizeof(buf), "%s - %s (attacker '%s', file '%s', line %u)", alertstring, error, ip_address, fname, lineno); } else { - fname = sapi_getenv("SCRIPT_FILENAME", 15 TSRMLS_CC); + fname = suhosin_getenv("SCRIPT_FILENAME", 15 TSRMLS_CC); if (fname==NULL) { fname = "unknown"; } diff --git a/php_suhosin.h b/php_suhosin.h index 1e2e053..3390094 100644 --- a/php_suhosin.h +++ b/php_suhosin.h @@ -306,6 +306,7 @@ char *suhosin_encrypt_string(char *str, int len, char *var, int vlen, char *key char *suhosin_decrypt_string(char *str, int padded_len, char *var, int vlen, char *key, int *orig_len, int check_ra TSRMLS_DC); char *suhosin_generate_key(char *key, zend_bool ua, zend_bool dr, long raddr, char *cryptkey TSRMLS_DC); char *suhosin_cookie_decryptor(TSRMLS_D); +char *suhosin_getenv(char *name, size_t name_len TSRMLS_DC); void suhosin_hook_post_handlers(TSRMLS_D); void suhosin_unhook_post_handlers(); void suhosin_hook_register_server_variables(); diff --git a/post_handler.c b/post_handler.c index c097a06..b405ae2 100644 --- a/post_handler.c +++ b/post_handler.c @@ -86,6 +86,40 @@ static void suhosin_post_handler_modification(sapi_post_entry *spe) efree(content_type); } +static int (*old_OnUpdate_mbstring_encoding_translation)(zend_ini_entry *entry, char *new_value, uint new_value_length, void *mh_arg1, void *mh_arg2, void *mh_arg3, int stage TSRMLS_DC) = NULL; + +/* {{{ static PHP_INI_MH(suhosin_OnUpdate_mbstring_encoding_translation) */ +static PHP_INI_MH(suhosin_OnUpdate_mbstring_encoding_translation) +{ + zend_bool *p; +#ifndef ZTS + char *base = (char *) mh_arg2; +#else + char *base; + + base = (char *) ts_resource(*((int *) mh_arg2)); +#endif + + p = (zend_bool *) (base+(size_t) mh_arg1); + + if (new_value_length == 2 && strcasecmp("on", new_value) == 0) { + *p = (zend_bool) 1; + } + else if (new_value_length == 3 && strcasecmp("yes", new_value) == 0) { + *p = (zend_bool) 1; + } + else if (new_value_length == 4 && strcasecmp("true", new_value) == 0) { + *p = (zend_bool) 1; + } + else { + *p = (zend_bool) atoi(new_value); + } + if (*p) { + suhosin_log(S_VARS, "Dynamic configuration (maybe a .htaccess file) tried to activate mbstring.encoding_translation which is incompatible with suhosin"); + } + return SUCCESS; +} +/* }}} */ /* {{{ php_post_entries[] */ @@ -99,6 +133,7 @@ static sapi_post_entry suhosin_post_entries[] = { void suhosin_hook_post_handlers(TSRMLS_D) { HashTable tempht; + zend_ini_entry *ini_entry; #if PHP_MAJOR_VERSION > 5 || (PHP_MAJOR_VERSION == 5 && PHP_MINOR_VERSION > 0) sapi_unregister_post_entry(&suhosin_post_entries[0] TSRMLS_CC); @@ -117,12 +152,30 @@ void suhosin_hook_post_handlers(TSRMLS_D) zend_hash_destroy(&tempht); /* And now we can overwrite the destructor for post entries */ SG(known_post_content_types).pDestructor = suhosin_post_handler_modification; + + /* we have to stop mbstring from replacing our post handler */ + if (zend_hash_find(EG(ini_directives), "mbstring.encoding_translation", sizeof("mbstring.encoding_translation"), (void **) &ini_entry) == FAILURE) { + return; + } + /* replace OnUpdate_mbstring_encoding_translation handler */ + old_OnUpdate_mbstring_encoding_translation = ini_entry->on_modify; + ini_entry->on_modify = suhosin_OnUpdate_mbstring_encoding_translation; } void suhosin_unhook_post_handlers() { + zend_ini_entry *ini_entry; + /* Restore to an empty destructor */ SG(known_post_content_types).pDestructor = NULL; + + /* Now restore the ini entry handler */ + if (zend_hash_find(EG(ini_directives), "mbstring.encoding_translation", sizeof("mbstring.encoding_translation"), (void **) &ini_entry) == FAILURE) { + return; + } + /* replace OnUpdate_mbstring_encoding_translation handler */ + ini_entry->on_modify = old_OnUpdate_mbstring_encoding_translation; + old_OnUpdate_mbstring_encoding_translation = NULL; } /* diff --git a/session.c b/session.c index 79aa11e..4786afa 100644 --- a/session.c +++ b/session.c @@ -371,7 +371,7 @@ static void suhosin_send_cookie(TSRMLS_D) void suhosin_get_ipv4(char *buf TSRMLS_DC) { - char *raddr = sapi_getenv("REMOTE_ADDR", sizeof("REMOTE_ADDR")-1 TSRMLS_CC); + char *raddr = suhosin_getenv("REMOTE_ADDR", sizeof("REMOTE_ADDR")-1 TSRMLS_CC); int i; @@ -573,15 +573,15 @@ char *suhosin_generate_key(char *key, zend_bool ua, zend_bool dr, long raddr, ch suhosin_SHA256_CTX ctx; if (ua) { - _ua = sapi_getenv("HTTP_USER_AGENT", sizeof("HTTP_USER_AGENT")-1 TSRMLS_CC); + _ua = suhosin_getenv("HTTP_USER_AGENT", sizeof("HTTP_USER_AGENT")-1 TSRMLS_CC); } if (dr) { - _dr = sapi_getenv("DOCUMENT_ROOT", sizeof("DOCUMENT_ROOT")-1 TSRMLS_CC); + _dr = suhosin_getenv("DOCUMENT_ROOT", sizeof("DOCUMENT_ROOT")-1 TSRMLS_CC); } if (raddr > 0) { - _ra = sapi_getenv("REMOTE_ADDR", sizeof("REMOTE_ADDR")-1 TSRMLS_CC); + _ra = suhosin_getenv("REMOTE_ADDR", sizeof("REMOTE_ADDR")-1 TSRMLS_CC); } SDEBUG("(suhosin_generate_key) KEY: %s - UA: %s - DR: %s - RA: %s", key,_ua,_dr,_ra); diff --git a/suhosin.c b/suhosin.c index e243bb2..cf2aae4 100644 --- a/suhosin.c +++ b/suhosin.c @@ -961,6 +961,34 @@ PHP_INI_END() /* }}} */ +/* {{{ suhosin_getenv + */ +char *suhosin_getenv(char *name, size_t name_len TSRMLS_DC) +{ + if (sapi_module.getenv) { + char *value, *tmp = sapi_module.getenv(name, name_len TSRMLS_CC); + if (tmp) { + value = estrdup(tmp); + } else { + return NULL; + } + return value; + } else { + /* fallback to the system's getenv() function */ + char *tmp; + + name = estrndup(name, name_len); + tmp = getenv(name); + efree(name); + if (tmp) { + return(estrdup(tmp)); + } + } + return NULL; +} +/* }}} */ + + /* {{{ suhosin_bailout */ void suhosin_bailout(TSRMLS_D) -- cgit v1.3