From 9d153cc185b4e2327a4aabe645cf1fabd3b4f21b Mon Sep 17 00:00:00 2001 From: jvoisin Date: Mon, 5 Feb 2018 18:13:57 +0100 Subject: Massive simplification of functions hooking --- src/sp_config.c | 2 +- src/sp_config_keywords.c | 38 +++++++++++++---------- src/sp_cookie_encryption.c | 3 +- src/sp_disable_xxe.c | 2 +- src/sp_disabled_functions.c | 14 ++++----- src/sp_harden_rand.c | 5 ++- src/sp_pcre_compat.c | 40 ++++++++++++------------ src/sp_unserialize.c | 5 ++- src/sp_utils.c | 43 ++++++++++---------------- src/sp_utils.h | 13 ++++---- src/sp_var_parser.c | 6 ++-- src/tests/config/disabled_functions_retval.ini | 1 + 12 files changed, 83 insertions(+), 89 deletions(-) (limited to 'src') diff --git a/src/sp_config.c b/src/sp_config.c index 1236ebd..67140a0 100644 --- a/src/sp_config.c +++ b/src/sp_config.c @@ -141,7 +141,7 @@ int parse_regexp(char *restrict line, char *restrict keyword, void *retval) { if (value) { sp_pcre *compiled_re = sp_pcre_compile(value); - if (NULL != compiled_re) { + if (NULL != compiled_re) { *(sp_pcre **)retval = compiled_re; return consumed; } diff --git a/src/sp_config_keywords.c b/src/sp_config_keywords.c index 5df3d97..959fa38 100644 --- a/src/sp_config_keywords.c +++ b/src/sp_config_keywords.c @@ -24,8 +24,8 @@ static int get_construct_type(sp_disabled_function const *const df) { return CONSTRUCTS_TYPES[i].type; } } else { - if (true == - sp_is_regexp_matching(df->r_function, CONSTRUCTS_TYPES[i].keys[j])) { + if (true == sp_is_regexp_matching(df->r_function, + CONSTRUCTS_TYPES[i].keys[j])) { return CONSTRUCTS_TYPES[i].type; } } @@ -122,13 +122,13 @@ static int parse_eval_filter_conf(char *line, sp_list_node **list) { } int parse_eval_blacklist(char *line) { - return parse_eval_filter_conf(line, - &SNUFFLEUPAGUS_G(config).config_eval->blacklist); + return parse_eval_filter_conf( + line, &SNUFFLEUPAGUS_G(config).config_eval->blacklist); } int parse_eval_whitelist(char *line) { - return parse_eval_filter_conf(line, - &SNUFFLEUPAGUS_G(config).config_eval->whitelist); + return parse_eval_filter_conf( + line, &SNUFFLEUPAGUS_G(config).config_eval->whitelist); } int parse_cookie(char *line) { @@ -204,8 +204,8 @@ int parse_cookie(char *line) { return -1; } } - SNUFFLEUPAGUS_G(config).config_cookie->cookies = sp_list_insert( - SNUFFLEUPAGUS_G(config).config_cookie->cookies, cookie); + SNUFFLEUPAGUS_G(config).config_cookie->cookies = + sp_list_insert(SNUFFLEUPAGUS_G(config).config_cookie->cookies, cookie); return SUCCESS; } @@ -338,9 +338,9 @@ int parse_disabled_functions(char *line) { if (param) { if (strlen(param) > 0 && param[0] != '$') { - /* This is an ugly hack. We're prefixing with a `$` because otherwise - * the parser treats this as a constant. - * FIXME: Remove this, and improve our (weird) parser. */ + /* This is an ugly hack. We're prefixing with a `$` because otherwise + * the parser treats this as a constant. + * FIXME: Remove this, and improve our (weird) parser. */ char *new = pecalloc(strlen(param) + 2, 1, 1); new[0] = '$'; memcpy(new + 1, param, strlen(param)); @@ -372,12 +372,14 @@ int parse_disabled_functions(char *line) { switch (get_construct_type(df)) { case ZEND_INCLUDE_OR_EVAL: - SNUFFLEUPAGUS_G(config).config_disabled_constructs->construct_include = sp_list_insert( + SNUFFLEUPAGUS_G(config) + .config_disabled_constructs->construct_include = sp_list_insert( SNUFFLEUPAGUS_G(config).config_disabled_constructs->construct_include, df); return ret; case ZEND_EVAL_CODE: - SNUFFLEUPAGUS_G(config).config_disabled_constructs->construct_eval = sp_list_insert( + SNUFFLEUPAGUS_G(config) + .config_disabled_constructs->construct_eval = sp_list_insert( SNUFFLEUPAGUS_G(config).config_disabled_constructs->construct_eval, df); return ret; @@ -391,11 +393,13 @@ int parse_disabled_functions(char *line) { } if (df->ret || df->r_ret || df->ret_type) { - SNUFFLEUPAGUS_G(config).config_disabled_functions_ret->disabled_functions = sp_list_insert( - SNUFFLEUPAGUS_G(config).config_disabled_functions_ret->disabled_functions, - df); + SNUFFLEUPAGUS_G(config).config_disabled_functions_ret->disabled_functions = + sp_list_insert(SNUFFLEUPAGUS_G(config) + .config_disabled_functions_ret->disabled_functions, + df); } else { - SNUFFLEUPAGUS_G(config).config_disabled_functions->disabled_functions = sp_list_insert( + SNUFFLEUPAGUS_G(config) + .config_disabled_functions->disabled_functions = sp_list_insert( SNUFFLEUPAGUS_G(config).config_disabled_functions->disabled_functions, df); } diff --git a/src/sp_cookie_encryption.c b/src/sp_cookie_encryption.c index 09cf884..4ecb97d 100644 --- a/src/sp_cookie_encryption.c +++ b/src/sp_cookie_encryption.c @@ -273,8 +273,7 @@ PHP_FUNCTION(sp_setcookie) { } int hook_cookies() { - HOOK_FUNCTION("setcookie", sp_internal_functions_hook, PHP_FN(sp_setcookie), - false); + HOOK_FUNCTION("setcookie", sp_internal_functions_hook, PHP_FN(sp_setcookie)); return SUCCESS; } diff --git a/src/sp_disable_xxe.c b/src/sp_disable_xxe.c index d11b3d0..df00dbd 100644 --- a/src/sp_disable_xxe.c +++ b/src/sp_disable_xxe.c @@ -19,7 +19,7 @@ int hook_libxml_disable_entity_loader() { call_user_function(CG(function_table), NULL, &func_name, &hmac, 1, params); HOOK_FUNCTION("libxml_disable_entity_loader", sp_internal_functions_hook, - PHP_FN(sp_libxml_disable_entity_loader), false); + PHP_FN(sp_libxml_disable_entity_loader)); return SUCCESS; } diff --git a/src/sp_disabled_functions.c b/src/sp_disabled_functions.c index f8c21d2..eb0ba83 100644 --- a/src/sp_disabled_functions.c +++ b/src/sp_disabled_functions.c @@ -258,8 +258,8 @@ bool should_disable(zend_execute_data* execute_data, const char* builtin_name, goto next; } } else if (config_node->r_function) { - if (false == - sp_is_regexp_matching(config_node->r_function, complete_path_function)) { + if (false == sp_is_regexp_matching(config_node->r_function, + complete_path_function)) { goto next; } } @@ -365,8 +365,8 @@ bool should_drop_on_ret(zval* return_value, goto next; } } else if (config_node->r_function) { - if (false == - sp_is_regexp_matching(config_node->r_function, complete_path_function)) { + if (false == sp_is_regexp_matching(config_node->r_function, + complete_path_function)) { goto next; } } @@ -445,10 +445,10 @@ static int hook_functions(const sp_list_node* config) { if (NULL != function_name) { // hook function by name HOOK_FUNCTION(function_name, disabled_functions_hook, - PHP_FN(check_disabled_function), false); + PHP_FN(check_disabled_function)); } else if (NULL != function_name_regexp) { // hook function by regexp HOOK_FUNCTION_BY_REGEXP(function_name_regexp, disabled_functions_hook, - PHP_FN(check_disabled_function), false); + PHP_FN(check_disabled_function)); } else { return FAILURE; } @@ -505,7 +505,7 @@ int hook_disabled_functions(void) { while (it) { hook_function((char*)it->data, SNUFFLEUPAGUS_G(sp_eval_blacklist_functions_hook), - PHP_FN(eval_blacklist_callback), false); + PHP_FN(eval_blacklist_callback)); it = it->next; } } diff --git a/src/sp_harden_rand.c b/src/sp_harden_rand.c index cb57591..0cb3058 100644 --- a/src/sp_harden_rand.c +++ b/src/sp_harden_rand.c @@ -79,9 +79,8 @@ PHP_FUNCTION(sp_mt_rand) { int hook_rand() { TSRMLS_FETCH(); - HOOK_FUNCTION("rand", sp_internal_functions_hook, PHP_FN(sp_rand), false); - HOOK_FUNCTION("mt_rand", sp_internal_functions_hook, PHP_FN(sp_mt_rand), - false); + HOOK_FUNCTION("rand", sp_internal_functions_hook, PHP_FN(sp_rand)); + HOOK_FUNCTION("mt_rand", sp_internal_functions_hook, PHP_FN(sp_mt_rand)); return SUCCESS; } diff --git a/src/sp_pcre_compat.c b/src/sp_pcre_compat.c index 42a11cb..c3f1d86 100644 --- a/src/sp_pcre_compat.c +++ b/src/sp_pcre_compat.c @@ -2,38 +2,40 @@ #include "sp_pcre_compat.h" -sp_pcre* sp_pcre_compile(const char *const pattern) { - sp_pcre* ret = NULL; - const char *pcre_error = NULL; +sp_pcre* sp_pcre_compile(const char* const pattern) { + sp_pcre* ret = NULL; + const char* pcre_error = NULL; #ifdef SP_HAS_PCRE2 - int errornumber; - PCRE2_SIZE erroroffset; - ret = pcre2_compile((PCRE2_SPTR)pattern, PCRE2_ZERO_TERMINATED, PCRE2_CASELESS, &errornumber, &erroroffset, NULL); + int errornumber; + PCRE2_SIZE erroroffset; + ret = pcre2_compile((PCRE2_SPTR)pattern, PCRE2_ZERO_TERMINATED, + PCRE2_CASELESS, &errornumber, &erroroffset, NULL); #else - int erroroffset; - ret = pcre_compile(pattern, PCRE_CASELESS, &pcre_error, &erroroffset, NULL); + int erroroffset; + ret = pcre_compile(pattern, PCRE_CASELESS, &pcre_error, &erroroffset, NULL); #endif - - if (NULL == ret) { - sp_log_err("config", "Failed to compile '%s': %s on line %zu.", pattern, - pcre_error, sp_line_no); - } - return ret; + + if (NULL == ret) { + sp_log_err("config", "Failed to compile '%s': %s on line %zu.", pattern, + pcre_error, sp_line_no); + } + return ret; } -bool sp_is_regexp_matching_len(const sp_pcre* regexp, const char* str, size_t len) { +bool sp_is_regexp_matching_len(const sp_pcre* regexp, const char* str, + size_t len) { int ret = 0; assert(NULL != regexp); assert(NULL != str); #ifdef SP_HAS_PCRE2 - pcre2_match_data *match_data = pcre2_match_data_create_from_pattern(regexp, NULL); - ret = pcre2_match(regexp, (PCRE2_SPTR)str, len, 0, 0, match_data, NULL); + pcre2_match_data* match_data = + pcre2_match_data_create_from_pattern(regexp, NULL); + ret = pcre2_match(regexp, (PCRE2_SPTR)str, len, 0, 0, match_data, NULL); #else int vec[30]; - ret = pcre_exec(regexp, NULL, str, len, 0, 0, vec, - sizeof(vec) / sizeof(int)); + ret = pcre_exec(regexp, NULL, str, len, 0, 0, vec, sizeof(vec) / sizeof(int)); #endif if (ret < 0) { diff --git a/src/sp_unserialize.c b/src/sp_unserialize.c index 6b7b03b..a1dbbee 100644 --- a/src/sp_unserialize.c +++ b/src/sp_unserialize.c @@ -102,10 +102,9 @@ PHP_FUNCTION(sp_unserialize) { int hook_serialize(void) { TSRMLS_FETCH(); - HOOK_FUNCTION("serialize", sp_internal_functions_hook, PHP_FN(sp_serialize), - false); + HOOK_FUNCTION("serialize", sp_internal_functions_hook, PHP_FN(sp_serialize)); HOOK_FUNCTION("unserialize", sp_internal_functions_hook, - PHP_FN(sp_unserialize), false); + PHP_FN(sp_unserialize)); return SUCCESS; } diff --git a/src/sp_utils.c b/src/sp_utils.c index 2979d98..8dbd14e 100644 --- a/src/sp_utils.c +++ b/src/sp_utils.c @@ -173,7 +173,8 @@ char* sp_convert_to_string(zval* zv) { return estrdup(""); } -bool sp_match_value(const char* value, const char* to_match, const sp_pcre* rx) { +bool sp_match_value(const char* value, const char* to_match, + const sp_pcre* rx) { if (to_match) { if (0 == strcmp(to_match, value)) { return true; @@ -255,7 +256,8 @@ void sp_log_disable_ret(const char* restrict path, } } -bool sp_match_array_key(const zval* zv, const char* to_match, const sp_pcre* rx) { +bool sp_match_array_key(const zval* zv, const char* to_match, + const sp_pcre* rx) { zend_string* key; zend_ulong idx; @@ -300,16 +302,14 @@ bool sp_match_array_value(const zval* arr, const char* to_match, } int hook_function(const char* original_name, HashTable* hook_table, - void (*new_function)(INTERNAL_FUNCTION_PARAMETERS), - bool hook_execution_table) { + void (*new_function)(INTERNAL_FUNCTION_PARAMETERS)) { zend_internal_function* func; - HashTable* ht = - hook_execution_table == true ? EG(function_table) : CG(function_table); /* The `mb` module likes to hook functions, like strlen->mb_strlen, * so we have to hook both of them. */ - if ((func = zend_hash_str_find_ptr(ht, VAR_AND_LEN(original_name)))) { + if ((func = zend_hash_str_find_ptr(CG(function_table), + VAR_AND_LEN(original_name)))) { if (func->handler == new_function) { return SUCCESS; } @@ -332,9 +332,9 @@ int hook_function(const char* original_name, HashTable* hook_table, if (0 == strncmp(original_name, "mb_", 3)) { CG(compiler_options) |= ZEND_COMPILE_NO_BUILTIN_STRLEN; - if (zend_hash_str_find(ht, VAR_AND_LEN(original_name + 3))) { - hook_function(original_name + 3, hook_table, new_function, - hook_execution_table); + if (zend_hash_str_find(CG(function_table), + VAR_AND_LEN(original_name + 3))) { + hook_function(original_name + 3, hook_table, new_function); } } else { // TODO this can be moved somewhere else to gain some marginal perfs CG(compiler_options) |= ZEND_COMPILE_NO_BUILTIN_STRLEN; @@ -342,7 +342,7 @@ int hook_function(const char* original_name, HashTable* hook_table, memcpy(mb_name, "mb_", 3); memcpy(mb_name + 3, VAR_AND_LEN(original_name)); if (zend_hash_str_find(CG(function_table), VAR_AND_LEN(mb_name))) { - hook_function(mb_name, hook_table, new_function, hook_execution_table); + hook_function(mb_name, hook_table, new_function); } } @@ -350,26 +350,17 @@ int hook_function(const char* original_name, HashTable* hook_table, } int hook_regexp(const sp_pcre* regexp, HashTable* hook_table, - void (*new_function)(INTERNAL_FUNCTION_PARAMETERS), - bool hook_execution_table) { + void (*new_function)(INTERNAL_FUNCTION_PARAMETERS)) { zend_string* key; - HashTable* ht = - hook_execution_table == true ? EG(function_table) : CG(function_table); - ZEND_HASH_FOREACH_STR_KEY(ht, key) { - if (key) { - int ret = sp_is_regexp_matching_len(regexp, key->val, key->len); - if (ret < 0) { /* Error or no match*/ - if (PCRE_ERROR_NOMATCH != ret) { - sp_log_err("pcre", "Runtime error with pcre, error code: %d", ret); - return FAILURE; - } - continue; - } - hook_function(key->val, hook_table, new_function, hook_execution_table); + ZEND_HASH_FOREACH_STR_KEY(CG(function_table), key) + if (key) { + if (true == sp_is_regexp_matching_len(regexp, key->val, key->len)) { + hook_function(key->val, hook_table, new_function); } } ZEND_HASH_FOREACH_END(); + return SUCCESS; } diff --git a/src/sp_utils.h b/src/sp_utils.h index 10a6daa..97808ad 100644 --- a/src/sp_utils.h +++ b/src/sp_utils.h @@ -22,12 +22,11 @@ #define SHA256_SIZE 32 -#define HOOK_FUNCTION(original_name, hook_table, new_function, execution) \ - hook_function(original_name, SNUFFLEUPAGUS_G(hook_table), new_function, \ - execution) +#define HOOK_FUNCTION(original_name, hook_table, new_function) \ + hook_function(original_name, SNUFFLEUPAGUS_G(hook_table), new_function) -#define HOOK_FUNCTION_BY_REGEXP(regexp, hook_table, new_function, execution) \ - hook_regexp(regexp, SNUFFLEUPAGUS_G(hook_table), new_function, execution) +#define HOOK_FUNCTION_BY_REGEXP(regexp, hook_table, new_function) \ + hook_regexp(regexp, SNUFFLEUPAGUS_G(hook_table), new_function) #define SP_LOG_SIMULATION "simulation" #define SP_LOG_DROP "drop" @@ -54,9 +53,9 @@ void sp_log_disable(const char *restrict, const char *restrict, void sp_log_disable_ret(const char *restrict, const char *restrict, const sp_disabled_function *); int hook_function(const char *, HashTable *, - void (*)(INTERNAL_FUNCTION_PARAMETERS), bool); + void (*)(INTERNAL_FUNCTION_PARAMETERS)); int hook_regexp(const sp_pcre *, HashTable *, - void (*)(INTERNAL_FUNCTION_PARAMETERS), bool); + void (*)(INTERNAL_FUNCTION_PARAMETERS)); bool check_is_in_eval_whitelist(const char * const function_name); #endif /* SP_UTILS_H */ diff --git a/src/sp_var_parser.c b/src/sp_var_parser.c index 330fa54..ab36677 100644 --- a/src/sp_var_parser.c +++ b/src/sp_var_parser.c @@ -1,7 +1,8 @@ #include "php_snuffleupagus.h" -static sp_list_node *parse_str_tokens(const char *str, const sp_conf_token token, - sp_list_node *tokens_list) { +static sp_list_node *parse_str_tokens(const char *str, + const sp_conf_token token, + sp_list_node *tokens_list) { const char *cur_str = str; while ((cur_str = strchr(cur_str, token.text_repr[0]))) { @@ -31,7 +32,6 @@ static bool is_var_name_valid(const char *name) { regexp_const = sp_pcre_compile(REGEXP_CONST); } if (NULL == regexp_var || NULL == regexp_const) { - sp_log_err("config", "Could not compile regexp."); return false; } if ((false == sp_is_regexp_matching(regexp_var, name)) && diff --git a/src/tests/config/disabled_functions_retval.ini b/src/tests/config/disabled_functions_retval.ini index b54c0fa..25a99f0 100644 --- a/src/tests/config/disabled_functions_retval.ini +++ b/src/tests/config/disabled_functions_retval.ini @@ -1 +1,2 @@ +sp.disable_function.function("str_repeat").ret("fufufu").filename("/var/www/test.php").drop(); sp.disable_function.function("str_repeat").ret("fufufu").drop(); -- cgit v1.3