From 206ffa3fb3fd72c6a2eb45194fb176535a91288c Mon Sep 17 00:00:00 2001 From: xXx-caillou-xXx Date: Thu, 30 Aug 2018 17:14:08 +0200 Subject: Minor code cleanup --- src/snuffleupagus.c | 196 ++++++++++++++++++++++---------------------- src/sp_config.c | 3 +- src/sp_config_keywords.c | 2 +- src/sp_cookie_encryption.c | 4 +- src/sp_crypt.c | 3 +- src/sp_disabled_functions.c | 10 +-- src/sp_execute.c | 81 +++++++++--------- src/sp_harden_rand.c | 4 +- src/sp_session.c | 9 +- src/sp_unserialize.c | 20 +++-- src/sp_upload_validation.c | 28 +++---- src/sp_utils.c | 15 ++-- 12 files changed, 186 insertions(+), 189 deletions(-) diff --git a/src/snuffleupagus.c b/src/snuffleupagus.c index 4fee81a..0126a37 100644 --- a/src/snuffleupagus.c +++ b/src/snuffleupagus.c @@ -51,12 +51,6 @@ PHP_INI_ENTRY("sp.allow_broken_configuration", "0", PHP_INI_SYSTEM, StrictMode) PHP_INI_END() -void free_disabled_functions_hashtable(HashTable *ht) { - void *ptr = NULL; - ZEND_HASH_FOREACH_PTR(ht, ptr) { sp_list_free(ptr); } - ZEND_HASH_FOREACH_END(); -} - ZEND_DLEXPORT zend_extension zend_extension_entry = { PHP_SNUFFLEUPAGUS_EXTNAME, PHP_SNUFFLEUPAGUS_VERSION, @@ -79,47 +73,45 @@ ZEND_DLEXPORT zend_extension zend_extension_entry = { PHP_GINIT_FUNCTION(snuffleupagus) { snuffleupagus_globals->in_eval = 0; -#define SP_INIT(F) F = pecalloc(sizeof(*F), 1, 1); -#define SP_INIT_HT(F) \ - F = pemalloc(sizeof(*F), 1); \ - zend_hash_init(F, 10, NULL, NULL, 1); - - SP_INIT_HT(snuffleupagus_globals->disabled_functions_hook); - SP_INIT_HT(snuffleupagus_globals->sp_internal_functions_hook); - SP_INIT_HT(snuffleupagus_globals->sp_eval_blacklist_functions_hook); - SP_INIT_HT(snuffleupagus_globals->config.config_disabled_functions); - SP_INIT_HT(snuffleupagus_globals->config.config_disabled_functions_hooked); - SP_INIT_HT(snuffleupagus_globals->config.config_disabled_functions_ret); - SP_INIT_HT( - snuffleupagus_globals->config.config_disabled_functions_ret_hooked); - - SP_INIT(snuffleupagus_globals->config.config_unserialize); - SP_INIT(snuffleupagus_globals->config.config_random); - SP_INIT(snuffleupagus_globals->config.config_sloppy); - SP_INIT(snuffleupagus_globals->config.config_readonly_exec); - SP_INIT(snuffleupagus_globals->config.config_global_strict); - SP_INIT(snuffleupagus_globals->config.config_auto_cookie_secure); - SP_INIT(snuffleupagus_globals->config.config_snuffleupagus); - SP_INIT(snuffleupagus_globals->config.config_disable_xxe); - SP_INIT(snuffleupagus_globals->config.config_upload_validation); - SP_INIT(snuffleupagus_globals->config.config_disabled_functions_reg); - SP_INIT(snuffleupagus_globals->config.config_disabled_functions_reg_ret); - SP_INIT(snuffleupagus_globals->config.config_cookie); - SP_INIT(snuffleupagus_globals->config.config_session); - SP_INIT(snuffleupagus_globals->config.config_eval); - SP_INIT(snuffleupagus_globals->config.config_wrapper); - - snuffleupagus_globals->config.config_disabled_functions_reg - ->disabled_functions = NULL; - snuffleupagus_globals->config.config_disabled_functions_reg_ret - ->disabled_functions = NULL; - snuffleupagus_globals->config.config_cookie->cookies = NULL; - snuffleupagus_globals->config.config_eval->blacklist = NULL; - snuffleupagus_globals->config.config_eval->whitelist = NULL; - snuffleupagus_globals->config.config_wrapper->whitelist = NULL; +#define SP_INIT_HT(F) snuffleupagus_globals->F = \ + pemalloc(sizeof(*(snuffleupagus_globals->F)), 1); \ + zend_hash_init(snuffleupagus_globals->F, 10, NULL, NULL, 1); + SP_INIT_HT(disabled_functions_hook); + SP_INIT_HT(sp_internal_functions_hook); + SP_INIT_HT(sp_eval_blacklist_functions_hook); + SP_INIT_HT(config.config_disabled_functions); + SP_INIT_HT(config.config_disabled_functions_hooked); + SP_INIT_HT(config.config_disabled_functions_ret); + SP_INIT_HT(config.config_disabled_functions_ret_hooked); +#undef SP_INIT_HT +#define SP_INIT(F) snuffleupagus_globals->config.F = \ + pecalloc(sizeof(*(snuffleupagus_globals->config.F)), 1, 1); + SP_INIT(config_unserialize); + SP_INIT(config_random); + SP_INIT(config_sloppy); + SP_INIT(config_readonly_exec); + SP_INIT(config_global_strict); + SP_INIT(config_auto_cookie_secure); + SP_INIT(config_snuffleupagus); + SP_INIT(config_disable_xxe); + SP_INIT(config_upload_validation); + SP_INIT(config_disabled_functions_reg); + SP_INIT(config_disabled_functions_reg_ret); + SP_INIT(config_cookie); + SP_INIT(config_session); + SP_INIT(config_eval); + SP_INIT(config_wrapper); #undef SP_INIT -#undef SP_INIT_HT + +#define SP_INIT_NULL(F) snuffleupagus_globals->config.F = NULL; + SP_INIT_NULL(config_disabled_functions_reg->disabled_functions); + SP_INIT_NULL(config_disabled_functions_reg_ret->disabled_functions); + SP_INIT_NULL(config_cookie->cookies); + SP_INIT_NULL(config_eval->blacklist); + SP_INIT_NULL(config_eval->whitelist); + SP_INIT_NULL(config_wrapper->whitelist); +#undef SP_INIT_NULL } PHP_MINIT_FUNCTION(snuffleupagus) { @@ -128,60 +120,62 @@ PHP_MINIT_FUNCTION(snuffleupagus) { return SUCCESS; } +static void free_disabled_functions_hashtable(HashTable *ht) { + void *ptr = NULL; + ZEND_HASH_FOREACH_PTR(ht, ptr) { sp_list_free(ptr); } + ZEND_HASH_FOREACH_END(); +} + PHP_MSHUTDOWN_FUNCTION(snuffleupagus) { - free_disabled_functions_hashtable( - SNUFFLEUPAGUS_G(config).config_disabled_functions); - free_disabled_functions_hashtable( - SNUFFLEUPAGUS_G(config).config_disabled_functions_hooked); - free_disabled_functions_hashtable( - SNUFFLEUPAGUS_G(config).config_disabled_functions_ret); - free_disabled_functions_hashtable( - SNUFFLEUPAGUS_G(config).config_disabled_functions_ret_hooked); #define FREE_HT(F) \ - zend_hash_destroy(SNUFFLEUPAGUS_G(F)); \ - pefree(SNUFFLEUPAGUS_G(F), 1); - + zend_hash_destroy(SNUFFLEUPAGUS_G(F)); \ + pefree(SNUFFLEUPAGUS_G(F), 1); FREE_HT(disabled_functions_hook); FREE_HT(sp_eval_blacklist_functions_hook); - FREE_HT(config.config_disabled_functions); - FREE_HT(config.config_disabled_functions_hooked); - FREE_HT(config.config_disabled_functions_ret); - FREE_HT(config.config_disabled_functions_ret_hooked); +#define FREE_HT_LIST(F) \ + free_disabled_functions_hashtable(SNUFFLEUPAGUS_G(config).F); \ + FREE_HT(config.F); + FREE_HT_LIST(config_disabled_functions); + FREE_HT_LIST(config_disabled_functions_hooked); + FREE_HT_LIST(config_disabled_functions_ret); + FREE_HT_LIST(config_disabled_functions_ret_hooked); +#undef FREE_HT_LIST #undef FREE_HT - pefree(SNUFFLEUPAGUS_G(config.config_unserialize), 1); - pefree(SNUFFLEUPAGUS_G(config.config_random), 1); - pefree(SNUFFLEUPAGUS_G(config.config_readonly_exec), 1); - pefree(SNUFFLEUPAGUS_G(config.config_global_strict), 1); - pefree(SNUFFLEUPAGUS_G(config.config_auto_cookie_secure), 1); - pefree(SNUFFLEUPAGUS_G(config.config_snuffleupagus), 1); - pefree(SNUFFLEUPAGUS_G(config.config_disable_xxe), 1); - pefree(SNUFFLEUPAGUS_G(config.config_upload_validation), 1); - pefree(SNUFFLEUPAGUS_G(config.config_session), 1); - #define FREE_LST_DISABLE(L) \ - do { \ - sp_list_node *_n = SNUFFLEUPAGUS_G(L); \ - sp_disabled_function_list_free(_n); \ - sp_list_free(_n); \ - } while (0) - - FREE_LST_DISABLE(config.config_disabled_functions_reg->disabled_functions); - FREE_LST_DISABLE( - config.config_disabled_functions_reg_ret->disabled_functions); - sp_list_free(SNUFFLEUPAGUS_G(config).config_cookie->cookies); - sp_list_free(SNUFFLEUPAGUS_G(config).config_eval->blacklist); - sp_list_free(SNUFFLEUPAGUS_G(config).config_eval->whitelist); - sp_list_free(SNUFFLEUPAGUS_G(config).config_wrapper->whitelist); - + do { \ + sp_list_node *_n = SNUFFLEUPAGUS_G(config).L; \ + sp_disabled_function_list_free(_n); \ + sp_list_free(_n); \ + } while (0) + FREE_LST_DISABLE(config_disabled_functions_reg->disabled_functions); + FREE_LST_DISABLE(config_disabled_functions_reg_ret->disabled_functions); #undef FREE_LST_DISABLE - pefree(SNUFFLEUPAGUS_G(config.config_disabled_functions_reg), 1); - pefree(SNUFFLEUPAGUS_G(config.config_disabled_functions_reg_ret), 1); - pefree(SNUFFLEUPAGUS_G(config.config_cookie), 1); - pefree(SNUFFLEUPAGUS_G(config.config_wrapper), 1); +#define FREE_LST(L) sp_list_free(SNUFFLEUPAGUS_G(config).L); + FREE_LST(config_cookie->cookies); + FREE_LST(config_eval->blacklist); + FREE_LST(config_eval->whitelist); + FREE_LST(config_wrapper->whitelist); +#undef FREE_LST + +#define FREE_CFG(C) pefree(SNUFFLEUPAGUS_G(config).C, 1); + FREE_CFG(config_unserialize); + FREE_CFG(config_random); + FREE_CFG(config_readonly_exec); + FREE_CFG(config_global_strict); + FREE_CFG(config_auto_cookie_secure); + FREE_CFG(config_snuffleupagus); + FREE_CFG(config_disable_xxe); + FREE_CFG(config_upload_validation); + FREE_CFG(config_session); + FREE_CFG(config_disabled_functions_reg); + FREE_CFG(config_disabled_functions_reg_ret); + FREE_CFG(config_cookie); + FREE_CFG(config_wrapper); +#undef FREE_CFG UNREGISTER_INI_ENTRIES(); @@ -189,6 +183,8 @@ PHP_MSHUTDOWN_FUNCTION(snuffleupagus) { } PHP_RINIT_FUNCTION(snuffleupagus) { + const sp_config_wrapper* config_wrapper = + SNUFFLEUPAGUS_G(config).config_wrapper; #if defined(COMPILE_DL_SNUFFLEUPAGUS) && defined(ZTS) ZEND_TSRMLS_CACHE_UPDATE(); #endif @@ -198,9 +194,9 @@ PHP_RINIT_FUNCTION(snuffleupagus) { } // We need to disable wrappers loaded by extensions loaded after SNUFFLEUPAGUS. - if (SNUFFLEUPAGUS_G(config).config_wrapper->enabled && + if (config_wrapper->enabled && zend_hash_num_elements(php_stream_get_url_stream_wrappers_hash()) != - SNUFFLEUPAGUS_G(config).config_wrapper->num_wrapper) { + config_wrapper->num_wrapper) { sp_disable_wrapper(); } @@ -265,28 +261,32 @@ static PHP_INI_MH(OnUpdateConfiguration) { if (SNUFFLEUPAGUS_G(config).config_random->enable) { hook_rand(); } + if (SNUFFLEUPAGUS_G(config).config_upload_validation->enable) { hook_upload(); } + if (SNUFFLEUPAGUS_G(config).config_disable_xxe->enable == 0) { hook_libxml_disable_entity_loader(); } + if (SNUFFLEUPAGUS_G(config).config_wrapper->enabled) { hook_stream_wrappers(); } - hook_disabled_functions(); - hook_execute(); + + if (SNUFFLEUPAGUS_G(config).config_session->encrypt) { + hook_session(); + } if (NULL != SNUFFLEUPAGUS_G(config).config_snuffleupagus->encryption_key) { if (SNUFFLEUPAGUS_G(config).config_unserialize->enable) { hook_serialize(); } } - hook_cookies(); - if (SNUFFLEUPAGUS_G(config).config_session->encrypt) { - hook_session(); - } + hook_disabled_functions(); + hook_execute(); + hook_cookies(); if (true == SNUFFLEUPAGUS_G(config).config_global_strict->enable) { if (!zend_get_extension(PHP_SNUFFLEUPAGUS_EXTNAME)) { @@ -300,10 +300,10 @@ static PHP_INI_MH(OnUpdateConfiguration) { // If `zend_write_default` is not NULL it is already hooked. if ((zend_hash_str_find( SNUFFLEUPAGUS_G(config).config_disabled_functions_hooked, "echo", - strlen("echo")) || + sizeof("echo") - 1) || zend_hash_str_find( SNUFFLEUPAGUS_G(config).config_disabled_functions_ret_hooked, "echo", - strlen("echo"))) && NULL == zend_write_default) { + sizeof("echo") - 1)) && NULL == zend_write_default) { zend_write_default = zend_write; zend_write = hook_echo; } diff --git a/src/sp_config.c b/src/sp_config.c index 2480362..dbf3881 100644 --- a/src/sp_config.c +++ b/src/sp_config.c @@ -113,8 +113,7 @@ int parse_php_type(char *restrict line, char *restrict keyword, void *retval) { sp_log_err("error", "%s) is expecting a valid php type ('false', 'true'," " 'array'. 'object', 'long', 'double', 'null', 'resource', " - "'reference'," - " 'undef') on line %zu", + "'reference', 'undef') on line %zu", keyword, sp_line_no); return -1; } diff --git a/src/sp_config_keywords.c b/src/sp_config_keywords.c index 93077c6..6bb7130 100644 --- a/src/sp_config_keywords.c +++ b/src/sp_config_keywords.c @@ -486,7 +486,7 @@ int parse_disabled_functions(char *line) { if (df->function && zend_string_equals_literal(df->function, "print")) { zend_string_release(df->function); - df->function = zend_string_init("echo", strlen("echo"), 1); + df->function = zend_string_init("echo", sizeof("echo") - 1, 1); } if (df->function && !df->functions_list) { diff --git a/src/sp_cookie_encryption.c b/src/sp_cookie_encryption.c index 1b59e44..31dde95 100644 --- a/src/sp_cookie_encryption.c +++ b/src/sp_cookie_encryption.c @@ -45,9 +45,9 @@ static zend_string *encrypt_data(zend_string *data) { PHP_FUNCTION(sp_setcookie) { zend_string *name = NULL, *value = NULL, *path = NULL, *domain = NULL, *value_enc = NULL, #if PHP_VERSION_ID < 70300 - *path_samesite = NULL; + *path_samesite = NULL; #else - *samesite = NULL; + *samesite = NULL; #endif zend_long expires = 0; diff --git a/src/sp_crypt.c b/src/sp_crypt.c index f4d1799..8336973 100644 --- a/src/sp_crypt.c +++ b/src/sp_crypt.c @@ -31,8 +31,7 @@ void generate_key(unsigned char *key) { "cookie_encryption", "The environment variable '%s' " "is empty, cookies are weakly encrypted", - ZSTR_VAL( - SNUFFLEUPAGUS_G(config).config_snuffleupagus->cookies_env_var)); + ZSTR_VAL(env_var_zend)); } if (encryption_key) { diff --git a/src/sp_disabled_functions.c b/src/sp_disabled_functions.c index 379ed75..835776b 100644 --- a/src/sp_disabled_functions.c +++ b/src/sp_disabled_functions.c @@ -580,13 +580,13 @@ ZEND_FUNCTION(eval_blacklist_callback) { if (SNUFFLEUPAGUS_G(in_eval) > 0) { zend_string* filename = get_eval_filename(zend_get_executed_filename()); const int line_number = zend_get_executed_lineno(TSRMLS_C); - if (SNUFFLEUPAGUS_G(config).config_eval->dump) { - sp_log_request( - SNUFFLEUPAGUS_G(config).config_eval->dump, - SNUFFLEUPAGUS_G(config).config_eval->textual_representation, + const sp_config_eval* config_eval = SNUFFLEUPAGUS_G(config).config_eval; + + if (config_eval->dump) { + sp_log_request(config_eval->dump, config_eval->textual_representation, SP_TOKEN_EVAL_BLACKLIST); } - if (SNUFFLEUPAGUS_G(config).config_eval->simulation) { + if (config_eval->simulation) { sp_log_msg("eval", SP_LOG_SIMULATION, "A call to %s was tried in eval, in %s:%d, logging it.", current_function_name, ZSTR_VAL(filename), line_number); diff --git a/src/sp_execute.c b/src/sp_execute.c index 5447ea1..60d63ab 100644 --- a/src/sp_execute.c +++ b/src/sp_execute.c @@ -13,14 +13,15 @@ static int (*orig_zend_stream_open)(const char *filename, // FIXME handle symlink ZEND_COLD static inline void terminate_if_writable(const char *filename) { + const sp_config_readonly_exec* config_ro_exec = + SNUFFLEUPAGUS_G(config).config_readonly_exec; + if (0 == access(filename, W_OK)) { - if (SNUFFLEUPAGUS_G(config).config_readonly_exec->dump) { - sp_log_request( - SNUFFLEUPAGUS_G(config).config_readonly_exec->dump, - SNUFFLEUPAGUS_G(config).config_readonly_exec->textual_representation, + if (config_ro_exec->dump) { + sp_log_request(config_ro_exec->dump, config_ro_exec->textual_representation, SP_TOKEN_READONLY_EXEC); } - if (true == SNUFFLEUPAGUS_G(config).config_readonly_exec->simulation) { + if (true == config_ro_exec->simulation) { sp_log_msg("readonly_exec", SP_LOG_SIMULATION, "Attempted execution of a writable file (%s).", filename); } else { @@ -57,7 +58,7 @@ inline static void is_builtin_matching( static void ZEND_HOT is_in_eval_and_whitelisted(const zend_execute_data *execute_data) { - sp_config_eval *eval = SNUFFLEUPAGUS_G(config).config_eval; + const sp_config_eval *config_eval = SNUFFLEUPAGUS_G(config).config_eval; if (EXPECTED(0 == SNUFFLEUPAGUS_G(in_eval))) { return; @@ -79,13 +80,11 @@ is_in_eval_and_whitelisted(const zend_execute_data *execute_data) { if (EXPECTED(NULL != current_function)) { if (UNEXPECTED(false == check_is_in_eval_whitelist(current_function))) { - if (eval->dump) { - sp_log_request( - SNUFFLEUPAGUS_G(config).config_eval->dump, - SNUFFLEUPAGUS_G(config).config_eval->textual_representation, + if (config_eval->dump) { + sp_log_request(config_eval->dump, config_eval->textual_representation, SP_TOKEN_EVAL_WHITELIST); } - if (eval->simulation) { + if (config_eval->simulation) { sp_log_msg( "Eval_whitelist", SP_LOG_SIMULATION, "The function '%s' isn't in the eval whitelist, logging its call.", @@ -124,17 +123,19 @@ zend_string *get_eval_filename(const char *const filename) { static void sp_execute_ex(zend_execute_data *execute_data) { is_in_eval_and_whitelisted(execute_data); + const HashTable* config_disabled_functions = + SNUFFLEUPAGUS_G(config).config_disabled_functions; if (!execute_data) { return; } if (UNEXPECTED(EX(func)->op_array.type == ZEND_EVAL_CODE)) { - const sp_list_node *config = zend_hash_str_find_ptr( - SNUFFLEUPAGUS_G(config).config_disabled_functions, "eval", 4); + const sp_list_node * config = zend_hash_str_find_ptr( + config_disabled_functions, "eval", sizeof("eval") - 1); + zend_string *filename = get_eval_filename(zend_get_executed_filename()); - is_builtin_matching(filename, "eval", NULL, config, - SNUFFLEUPAGUS_G(config).config_disabled_functions); + is_builtin_matching(filename, "eval", NULL, config, config_disabled_functions); zend_string_release(filename); SNUFFLEUPAGUS_G(in_eval)++; @@ -152,6 +153,9 @@ static void sp_execute_ex(zend_execute_data *execute_data) { if (SNUFFLEUPAGUS_G(config).hook_execute) { char *function_name = get_complete_function_path(execute_data); zval ret_val; + const sp_list_node* config_disabled_functions_reg = + SNUFFLEUPAGUS_G(config).config_disabled_functions_reg + ->disabled_functions; if (!function_name) { orig_execute_ex(execute_data); @@ -163,11 +167,9 @@ static void sp_execute_ex(zend_execute_data *execute_data) { !ZEND_USER_CODE(execute_data->prev_execute_data->func->type) || !execute_data->prev_execute_data->opline) { if (UNEXPECTED(true == - should_disable_ht( - execute_data, function_name, NULL, NULL, - SNUFFLEUPAGUS_G(config) - .config_disabled_functions_reg->disabled_functions, - SNUFFLEUPAGUS_G(config).config_disabled_functions))) { + should_disable_ht(execute_data, function_name, NULL, NULL, + config_disabled_functions_reg, + config_disabled_functions))) { sp_terminate(); } } else if ((execute_data->prev_execute_data->opline->opcode == @@ -177,11 +179,9 @@ static void sp_execute_ex(zend_execute_data *execute_data) { execute_data->prev_execute_data->opline->opcode == ZEND_DO_FCALL_BY_NAME)) { if (UNEXPECTED(true == - should_disable_ht( - execute_data, function_name, NULL, NULL, - SNUFFLEUPAGUS_G(config) - .config_disabled_functions_reg->disabled_functions, - SNUFFLEUPAGUS_G(config).config_disabled_functions))) { + should_disable_ht(execute_data, function_name, NULL, NULL, + config_disabled_functions_reg, + config_disabled_functions))) { sp_terminate(); } } @@ -235,6 +235,9 @@ static int sp_stream_open(const char *filename, zend_file_handle *handle) { } zend_string *zend_filename = zend_string_init(filename, strlen(filename), 0); + const HashTable* disabled_functions_hooked = + SNUFFLEUPAGUS_G(config).config_disabled_functions_hooked; + switch (data->opline->opcode) { case ZEND_INCLUDE_OR_EVAL: if (true == SNUFFLEUPAGUS_G(config).config_readonly_exec->enable) { @@ -244,34 +247,30 @@ static int sp_stream_open(const char *filename, zend_file_handle *handle) { case ZEND_INCLUDE: is_builtin_matching( zend_filename, "include", "inclusion path", - zend_hash_str_find_ptr( - SNUFFLEUPAGUS_G(config).config_disabled_functions_hooked, - "include", 7), - SNUFFLEUPAGUS_G(config).config_disabled_functions_hooked); + zend_hash_str_find_ptr(disabled_functions_hooked, + "include", sizeof("include") - 1), + disabled_functions_hooked); break; case ZEND_REQUIRE: is_builtin_matching( zend_filename, "require", "inclusion path", - zend_hash_str_find_ptr( - SNUFFLEUPAGUS_G(config).config_disabled_functions_hooked, - "require", 7), - SNUFFLEUPAGUS_G(config).config_disabled_functions_hooked); + zend_hash_str_find_ptr(disabled_functions_hooked, + "require", sizeof("require") - 1), + disabled_functions_hooked); break; case ZEND_REQUIRE_ONCE: is_builtin_matching( zend_filename, "require_once", "inclusion path", - zend_hash_str_find_ptr( - SNUFFLEUPAGUS_G(config).config_disabled_functions_hooked, - "require_once", 12), - SNUFFLEUPAGUS_G(config).config_disabled_functions_hooked); + zend_hash_str_find_ptr(disabled_functions_hooked, + "require_once", sizeof("require_once") - 1), + disabled_functions_hooked); break; case ZEND_INCLUDE_ONCE: is_builtin_matching( zend_filename, "include_once", "inclusion path", - zend_hash_str_find_ptr( - SNUFFLEUPAGUS_G(config).config_disabled_functions_hooked, - "include_once", 12), - SNUFFLEUPAGUS_G(config).config_disabled_functions_hooked); + zend_hash_str_find_ptr(disabled_functions_hooked, + "include_once", sizeof("include_once") - 1), + disabled_functions_hooked); break; EMPTY_SWITCH_DEFAULT_CASE(); } diff --git a/src/sp_harden_rand.c b/src/sp_harden_rand.c index ca0503b..7b4e958 100644 --- a/src/sp_harden_rand.c +++ b/src/sp_harden_rand.c @@ -57,7 +57,7 @@ PHP_FUNCTION(sp_rand) { /* call the original `rand` function, * since we might no be the only ones to hook it*/ orig_handler = zend_hash_str_find_ptr( - SNUFFLEUPAGUS_G(sp_internal_functions_hook), "rand", strlen("rand")); + SNUFFLEUPAGUS_G(sp_internal_functions_hook), "rand", sizeof("rand") - 1); orig_handler(INTERNAL_FUNCTION_PARAM_PASSTHRU); random_int_wrapper(INTERNAL_FUNCTION_PARAM_PASSTHRU); @@ -70,7 +70,7 @@ PHP_FUNCTION(sp_mt_rand) { * since we might no be the only ones to hook it*/ orig_handler = zend_hash_str_find_ptr(SNUFFLEUPAGUS_G(sp_internal_functions_hook), - "mt_rand", strlen("mt_rand")); + "mt_rand", sizeof("mt_rand") - 1); orig_handler(INTERNAL_FUNCTION_PARAM_PASSTHRU); random_int_wrapper(INTERNAL_FUNCTION_PARAM_PASSTHRU); diff --git a/src/sp_session.c b/src/sp_session.c index 35866a1..8aad624 100644 --- a/src/sp_session.c +++ b/src/sp_session.c @@ -23,16 +23,17 @@ static ZEND_INI_MH((*old_OnUpdateSaveHandler)) = NULL; static int sp_hook_s_read(PS_READ_ARGS) { int r = old_s_read(mod_data, key, val, maxlifetime); - if (r == SUCCESS && SNUFFLEUPAGUS_G(config).config_session->encrypt && + const sp_config_session* config_session = SNUFFLEUPAGUS_G(config).config_session; + + if (r == SUCCESS && config_session->encrypt && val != NULL && *val != NULL && ZSTR_LEN(*val)) { zend_string *orig_val = *val; zval val_zval; ZVAL_PSTRINGL(&val_zval, ZSTR_VAL(*val), ZSTR_LEN(*val)); - int ret = decrypt_zval( - &val_zval, SNUFFLEUPAGUS_G(config).config_session->simulation, NULL); + int ret = decrypt_zval(&val_zval, config_session->simulation, NULL); if (0 != ret) { - if (SNUFFLEUPAGUS_G(config).config_session->simulation) { + if (config_session->simulation) { return ret; } else { sp_terminate(); diff --git a/src/sp_unserialize.c b/src/sp_unserialize.c index 9ed1c55..ab0139a 100644 --- a/src/sp_unserialize.c +++ b/src/sp_unserialize.c @@ -7,7 +7,8 @@ PHP_FUNCTION(sp_serialize) { /* Call the original `serialize` function. */ orig_handler = zend_hash_str_find_ptr( - SNUFFLEUPAGUS_G(sp_internal_functions_hook), "serialize", 9); + SNUFFLEUPAGUS_G(sp_internal_functions_hook), "serialize", + sizeof("serialize") - 1); orig_handler(INTERNAL_FUNCTION_PARAM_PASSTHRU); /* Compute the HMAC of the textual representation of the serialized data*/ @@ -50,6 +51,9 @@ PHP_FUNCTION(sp_unserialize) { size_t buf_len = 0; zval *opts = NULL; + const sp_config_unserialize* config_unserialize = + SNUFFLEUPAGUS_G(config).config_unserialize; + if (zend_parse_parameters(ZEND_NUM_ARGS(), "s|a", &buf, &buf_len, &opts) == FAILURE) { RETURN_FALSE; @@ -85,16 +89,17 @@ PHP_FUNCTION(sp_unserialize) { if (0 == status) { if ((orig_handler = zend_hash_str_find_ptr( - SNUFFLEUPAGUS_G(sp_internal_functions_hook), "unserialize", 11))) { + SNUFFLEUPAGUS_G(sp_internal_functions_hook), "unserialize", + sizeof("unserialize") - 1))) { orig_handler(INTERNAL_FUNCTION_PARAM_PASSTHRU); } } else { - if (true == SNUFFLEUPAGUS_G(config).config_unserialize->simulation) { + if (true == config_unserialize->simulation) { sp_log_msg("unserialize", SP_LOG_SIMULATION, "Invalid HMAC for %s", serialized_str); if ((orig_handler = zend_hash_str_find_ptr( SNUFFLEUPAGUS_G(sp_internal_functions_hook), "unserialize", - 11))) { + sizeof("unserialize") - 1))) { orig_handler(INTERNAL_FUNCTION_PARAM_PASSTHRU); } } else { @@ -102,10 +107,9 @@ PHP_FUNCTION(sp_unserialize) { serialized_str); } } - if (SNUFFLEUPAGUS_G(config).config_unserialize->dump) { - sp_log_request( - SNUFFLEUPAGUS_G(config).config_unserialize->dump, - SNUFFLEUPAGUS_G(config).config_unserialize->textual_representation, + if (config_unserialize->dump) { + sp_log_request(config_unserialize->dump, + config_unserialize->textual_representation, SP_TOKEN_UNSERIALIZE_HMAC); } efree(serialized_str); diff --git a/src/sp_upload_validation.c b/src/sp_upload_validation.c index c079e41..6fa721e 100644 --- a/src/sp_upload_validation.c +++ b/src/sp_upload_validation.c @@ -21,22 +21,23 @@ int sp_rfc1867_callback(unsigned int event, void *event_data, void **extra) { if (event == MULTIPART_EVENT_END) { zend_string *file_key __attribute__((unused)) = NULL; + const sp_config_upload_validation* config_upload = + SNUFFLEUPAGUS_G(config).config_upload_validation; zval *file; pid_t pid; - sp_log_debug( - "Got %d files", + sp_log_debug("Got %d files", zend_hash_num_elements(Z_ARRVAL(PG(http_globals)[TRACK_VARS_FILES]))); ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL(PG(http_globals)[TRACK_VARS_FILES]), file_key, file) { // for each uploaded file char *filename = - Z_STRVAL_P(zend_hash_str_find(Z_ARRVAL_P(file), "name", 4)); + Z_STRVAL_P(zend_hash_str_find(Z_ARRVAL_P(file), "name", sizeof("name") - 1)); char *tmp_name = - Z_STRVAL_P(zend_hash_str_find(Z_ARRVAL_P(file), "tmp_name", 8)); + Z_STRVAL_P(zend_hash_str_find(Z_ARRVAL_P(file), "tmp_name", sizeof("tmp_name") - 1)); size_t filesize = - Z_LVAL_P(zend_hash_str_find(Z_ARRVAL_P(file), "size", 4)); + Z_LVAL_P(zend_hash_str_find(Z_ARRVAL_P(file), "size", sizeof("size") - 1)); char *cmd[3] = {0}; char *env[5] = {0}; @@ -44,10 +45,9 @@ int sp_rfc1867_callback(unsigned int event, void *event_data, void **extra) { "Filename: %s\nTmpname: %s\nSize: %d\nError: %d\nScript: %s", filename, tmp_name, filesize, Z_LVAL_P(zend_hash_str_find(Z_ARRVAL_P(file), "error", 5)), - ZSTR_VAL(SNUFFLEUPAGUS_G(config).config_upload_validation->script)); + ZSTR_VAL(config_upload->script)); - cmd[0] = - ZSTR_VAL(SNUFFLEUPAGUS_G(config).config_upload_validation->script); + cmd[0] = ZSTR_VAL(config_upload->script); cmd[1] = tmp_name; cmd[2] = NULL; @@ -59,14 +59,10 @@ int sp_rfc1867_callback(unsigned int event, void *event_data, void **extra) { env[4] = NULL; if ((pid = fork()) == 0) { - if (execve( - ZSTR_VAL( - SNUFFLEUPAGUS_G(config).config_upload_validation->script), - cmd, env) == -1) { + if (execve(ZSTR_VAL(config_upload->script), cmd, env) == -1) { sp_log_warn( "upload_validation", "Could not call '%s' : %s", - ZSTR_VAL( - SNUFFLEUPAGUS_G(config).config_upload_validation->script), + ZSTR_VAL(config_upload->script), strerror(errno)); EFREE_3(env); exit(1); @@ -85,11 +81,11 @@ int sp_rfc1867_callback(unsigned int event, void *event_data, void **extra) { wait(&waitstatus); if (WEXITSTATUS(waitstatus) != 0) { // Nope char *uri = getenv("REQUEST_URI"); - int sim = SNUFFLEUPAGUS_G(config).config_upload_validation->simulation; + int sim = config_upload->simulation; sp_log_msg("upload_validation", sim ? SP_LOG_SIMULATION : SP_LOG_DROP, "The upload of %s on %s was rejected.", filename, uri ? uri : "?"); - if (!SNUFFLEUPAGUS_G(config).config_upload_validation->simulation) { + if (!config_upload->simulation) { sp_terminate(); } } diff --git a/src/sp_utils.c b/src/sp_utils.c index e872abd..970f314 100644 --- a/src/sp_utils.c +++ b/src/sp_utils.c @@ -126,7 +126,6 @@ int sp_log_request(const zend_string* folder, const zend_string* text_repr, } fclose(file); -#undef CAT_AND_DEC return 0; } @@ -171,17 +170,17 @@ const zend_string* sp_zval_to_zend_string(const zval* zv) { return Z_STR_P(zv); } case IS_FALSE: - return zend_string_init("FALSE", 5, 0); + return zend_string_init("FALSE", sizeof("FALSE") - 1, 0); case IS_TRUE: - return zend_string_init("TRUE", 4, 0); + return zend_string_init("TRUE", sizeof("TRUE") - 1, 0); case IS_NULL: - return zend_string_init("NULL", 4, 0); + return zend_string_init("NULL", sizeof("NULL") - 1, 0); case IS_OBJECT: - return zend_string_init("OBJECT", 6, 0); + return zend_string_init("OBJECT", sizeof("OBJECT") - 1, 0); case IS_ARRAY: - return zend_string_init("ARRAY", 5, 0); + return zend_string_init("ARRAY", sizeof("ARRAY") - 1, 0); case IS_RESOURCE: - return zend_string_init("RESOURCE", 8, 0); + return zend_string_init("RESOURCE", sizeof("RESOURCE") - 1, 0); } return zend_string_init("", 0, 0); } @@ -353,7 +352,7 @@ int hook_function(const char* original_name, HashTable* hook_table, } else { // TODO this can be moved somewhere else to gain some marginal perfs CG(compiler_options) |= ZEND_COMPILE_NO_BUILTIN_STRLEN; char* mb_name = ecalloc(strlen(original_name) + 3 + 1, 1); - memcpy(mb_name, "mb_", 3); + memcpy(mb_name, "mb_", sizeof("mb_") - 1); memcpy(mb_name + 3, VAR_AND_LEN(original_name)); if (zend_hash_str_find(CG(function_table), VAR_AND_LEN(mb_name))) { return hook_function(mb_name, hook_table, new_function); -- cgit v1.3