From 654552c14ba8c98a244f82f9b8f1225a68526efb Mon Sep 17 00:00:00 2001 From: Julien Voisin Date: Thu, 25 Mar 2021 18:55:27 +0000 Subject: Add PHP8 for linux distributions on the CI --- .../broken_conf_session_encryption_without_encryption_key.phpt | 1 + .../broken_conf_session_encryption_without_env_var.phpt | 1 + 2 files changed, 2 insertions(+) (limited to 'src') diff --git a/src/tests/broken_configuration_php8/broken_conf_session_encryption_without_encryption_key.phpt b/src/tests/broken_configuration_php8/broken_conf_session_encryption_without_encryption_key.phpt index 046dc7d..62ee41e 100644 --- a/src/tests/broken_configuration_php8/broken_conf_session_encryption_without_encryption_key.phpt +++ b/src/tests/broken_configuration_php8/broken_conf_session_encryption_without_encryption_key.phpt @@ -6,6 +6,7 @@ Broken configuration - encrypted session without encryption key --INI-- sp.configuration_file={PWD}/config/broken_conf_session_encryption_without_encryption_key.ini --FILE-- +--XFAIL-- --EXPECT-- Fatal error: [snuffleupagus][0.0.0.0][config][log] You're trying to use the session cookie encryption feature on line 2 without having set the `.secret_key` option in`sp.global`: please set it first in Unknown on line 0 diff --git a/src/tests/broken_configuration_php8/broken_conf_session_encryption_without_env_var.phpt b/src/tests/broken_configuration_php8/broken_conf_session_encryption_without_env_var.phpt index bb0f212..5acc1cd 100644 --- a/src/tests/broken_configuration_php8/broken_conf_session_encryption_without_env_var.phpt +++ b/src/tests/broken_configuration_php8/broken_conf_session_encryption_without_env_var.phpt @@ -6,6 +6,7 @@ Broken configuration - encrypted session without env var --INI-- sp.configuration_file={PWD}/config/broken_conf_session_encryption_without_env_var.ini --FILE-- +--XFAIL-- --EXPECT-- Fatal error: [snuffleupagus][0.0.0.0][config][log] You're trying to use the session cookie encryption feature on line 2 without having set the `.cookie_env_var` option in`sp.global`: please set it first in Unknown on line 0 -- cgit v1.3 From 7fc7743977905807b4c2fdcde183a219ace25ba9 Mon Sep 17 00:00:00 2001 From: Julien Voisin Date: Mon, 26 Apr 2021 22:25:50 +0200 Subject: Make it easier to figure functions parameters' names --- src/sp_var_value.c | 9 +++++++-- .../broken_configuration/broken_conf_config_invalid_param.phpt | 6 ++++++ 2 files changed, 13 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/sp_var_value.c b/src/sp_var_value.c index b9ac357..e351446 100644 --- a/src/sp_var_value.c +++ b/src/sp_var_value.c @@ -1,6 +1,7 @@ #include "php_snuffleupagus.h" -static zval *get_param_var(zend_execute_data *ed, const char *var_name) { +static zval *get_param_var(zend_execute_data *ed, const char *var_name, + bool print) { unsigned int nb_param = ed->func->common.num_args; for (unsigned int i = 0; i < nb_param; i++) { @@ -13,6 +14,9 @@ static zval *get_param_var(zend_execute_data *ed, const char *var_name) { if (0 == strcmp(arg_name, var_name)) { return ZEND_CALL_VAR_NUM(ed, i); } + if (print == true) { + sp_log_warn("config", " - %d parameter's name: '%s'", i, arg_name); + } } return NULL; } @@ -68,7 +72,7 @@ static zval *get_var_value(zend_execute_data *ed, const char *var_name, } if (is_param) { - zval *zvalue = get_param_var(ed, var_name); + zval *zvalue = get_param_var(ed, var_name, false); if (!zvalue) { char *complete_function_path = get_complete_function_path(ed); sp_log_warn("config", @@ -76,6 +80,7 @@ static zval *get_var_value(zend_execute_data *ed, const char *var_name, "'%s' of the function '%s', but the parameter does " "not exists.", var_name, complete_function_path); + get_param_var(ed, var_name, true); efree(complete_function_path); return NULL; } diff --git a/src/tests/broken_configuration/broken_conf_config_invalid_param.phpt b/src/tests/broken_configuration/broken_conf_config_invalid_param.phpt index ac85dea..45ccf24 100644 --- a/src/tests/broken_configuration/broken_conf_config_invalid_param.phpt +++ b/src/tests/broken_configuration/broken_conf_config_invalid_param.phpt @@ -13,4 +13,10 @@ function foo($blah, $x = null, $y = null) { foo("qwe"); --EXPECTF-- Warning: [snuffleupagus][0.0.0.0][config][log] It seems that you are filtering on a parameter 'qwe' of the function 'foo', but the parameter does not exists. in %s/tests/broken_configuration/broken_conf_config_invalid_param.php on line %d + +Warning: [snuffleupagus][0.0.0.0][config][log] - 0 parameter's name: 'blah' in %s/tests/broken_configuration/broken_conf_config_invalid_param.php on line %d + +Warning: [snuffleupagus][0.0.0.0][config][log] - 1 parameter's name: 'x' in %s/tests/broken_configuration/broken_conf_config_invalid_param.php on line %d + +Warning: [snuffleupagus][0.0.0.0][config][log] - 2 parameter's name: 'y' in %s/tests/broken_configuration/broken_conf_config_invalid_param.php on line %d ok -- cgit v1.3 From 24e3f3d80a62fc32b986a2493d4d85be9aa6a6e2 Mon Sep 17 00:00:00 2001 From: Tristan Deloche Date: Tue, 27 Apr 2021 19:39:36 +0100 Subject: Fix SKIPIF output syntax error --- src/tests/deny_writable/deny_writable_execution_simulation.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/tests/deny_writable/deny_writable_execution_simulation.phpt b/src/tests/deny_writable/deny_writable_execution_simulation.phpt index 30f8cb1..d4b8efc 100644 --- a/src/tests/deny_writable/deny_writable_execution_simulation.phpt +++ b/src/tests/deny_writable/deny_writable_execution_simulation.phpt @@ -3,7 +3,7 @@ Readonly execution attempt (simulation mode) --SKIPIF-- = 80000) print "skip"; ?> getElementsByTagName('testing')->item(0)->nodeVa ?> --EXPECTF-- +Warning: [snuffleupagus][0.0.0.0][xxe][log] A call to libxml_disable_entity_loader was tried and nopped in %s/tests/xxe/disable_xxe_dom_disabled.php on line %d libxml_disable_entity to true: WARNING, external entity loaded! + +Warning: [snuffleupagus][0.0.0.0][xxe][log] A call to libxml_disable_entity_loader was tried and nopped in %s/tests/xxe/disable_xxe_dom_disabled.php on line %d libxml_disable_entity to false: WARNING, external entity loaded! + +Warning: [snuffleupagus][0.0.0.0][xxe][log] A call to libxml_disable_entity_loader was tried and nopped in %s/tests/xxe/disable_xxe_dom_disabled.php on line %d without xxe: foo --CLEAN-- is_config_valid = SP_CONFIG_NONE; snuffleupagus_globals->in_eval = 0; -#define SP_INIT_HT(F) snuffleupagus_globals->F = \ - pemalloc(sizeof(*(snuffleupagus_globals->F)), 1); \ - zend_hash_init(snuffleupagus_globals->F, 10, NULL, NULL, 1); +#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); @@ -86,8 +85,9 @@ PHP_GINIT_FUNCTION(snuffleupagus) { 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); +#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); @@ -128,16 +128,15 @@ static void free_disabled_functions_hashtable(HashTable *ht) { } PHP_MSHUTDOWN_FUNCTION(snuffleupagus) { - #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); -#define FREE_HT_LIST(F) \ - free_disabled_functions_hashtable(SNUFFLEUPAGUS_G(config).F); \ - FREE_HT(config.F); +#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); @@ -145,12 +144,12 @@ PHP_MSHUTDOWN_FUNCTION(snuffleupagus) { #undef FREE_HT_LIST #undef FREE_HT -#define FREE_LST_DISABLE(L) \ - do { \ - sp_list_node *_n = SNUFFLEUPAGUS_G(config).L; \ - sp_disabled_function_list_free(_n); \ - sp_list_free(_n); \ - } while (0) +#define FREE_LST_DISABLE(L) \ + 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 @@ -184,24 +183,26 @@ PHP_MSHUTDOWN_FUNCTION(snuffleupagus) { } PHP_RINIT_FUNCTION(snuffleupagus) { - const sp_config_wrapper* config_wrapper = + const sp_config_wrapper *config_wrapper = SNUFFLEUPAGUS_G(config).config_wrapper; #if defined(COMPILE_DL_SNUFFLEUPAGUS) && defined(ZTS) ZEND_TSRMLS_CACHE_UPDATE(); #endif if (!SNUFFLEUPAGUS_G(allow_broken_configuration)) { - if (SNUFFLEUPAGUS_G(is_config_valid) == SP_CONFIG_INVALID ) { + if (SNUFFLEUPAGUS_G(is_config_valid) == SP_CONFIG_INVALID) { sp_log_err("config", "Invalid configuration file"); } else if (SNUFFLEUPAGUS_G(is_config_valid) == SP_CONFIG_NONE) { - sp_log_warn("config", "No configuration specificed via sp.configuration_file"); + sp_log_warn("config", + "No configuration specificed via sp.configuration_file"); } } - // We need to disable wrappers loaded by extensions loaded after SNUFFLEUPAGUS. + // We need to disable wrappers loaded by extensions loaded after + // SNUFFLEUPAGUS. if (config_wrapper->enabled && zend_hash_num_elements(php_stream_get_url_stream_wrappers_hash()) != - config_wrapper->num_wrapper) { + config_wrapper->num_wrapper) { sp_disable_wrapper(); } @@ -218,7 +219,7 @@ PHP_RSHUTDOWN_FUNCTION(snuffleupagus) { return SUCCESS; } PHP_MINFO_FUNCTION(snuffleupagus) { const char *valid_config; - switch(SNUFFLEUPAGUS_G(is_config_valid)) { + switch (SNUFFLEUPAGUS_G(is_config_valid)) { case SP_CONFIG_VALID: valid_config = "yes"; break; @@ -230,10 +231,11 @@ PHP_MINFO_FUNCTION(snuffleupagus) { valid_config = "no"; } php_info_print_table_start(); - php_info_print_table_row(2, "snuffleupagus support", - SNUFFLEUPAGUS_G(is_config_valid)?"enabled":"disabled"); + php_info_print_table_row( + 2, "snuffleupagus support", + SNUFFLEUPAGUS_G(is_config_valid) ? "enabled" : "disabled"); php_info_print_table_row(2, "Version", PHP_SNUFFLEUPAGUS_VERSION); - php_info_print_table_row( 2, "Valid config", valid_config); + php_info_print_table_row(2, "Valid config", valid_config); php_info_print_table_end(); DISPLAY_INI_ENTRIES(); } @@ -315,11 +317,12 @@ 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", - sizeof("echo") - 1) || - zend_hash_str_find( - SNUFFLEUPAGUS_G(config).config_disabled_functions_ret_hooked, "echo", - sizeof("echo") - 1)) && NULL == zend_write_default) { + SNUFFLEUPAGUS_G(config).config_disabled_functions_hooked, "echo", + sizeof("echo") - 1) || + zend_hash_str_find( + SNUFFLEUPAGUS_G(config).config_disabled_functions_ret_hooked, "echo", + sizeof("echo") - 1)) && + NULL == zend_write_default) { zend_write_default = zend_write; zend_write = hook_echo; } diff --git a/src/sp_disable_xxe.c b/src/sp_disable_xxe.c index 9dea33c..f9712b5 100644 --- a/src/sp_disable_xxe.c +++ b/src/sp_disable_xxe.c @@ -1,12 +1,15 @@ #include "php_snuffleupagus.h" PHP_FUNCTION(sp_libxml_disable_entity_loader) { - sp_log_warn( "xxe", "A call to libxml_disable_entity_loader was tried and nopped"); + sp_log_warn("xxe", + "A call to libxml_disable_entity_loader was tried and nopped"); RETURN_TRUE; } PHP_FUNCTION(sp_libxml_set_external_entity_loader) { - sp_log_warn("xxe", "A call to libxml_set_external_entity_loader was tried and nopped"); + sp_log_warn( + "xxe", + "A call to libxml_set_external_entity_loader was tried and nopped"); RETURN_TRUE; } diff --git a/src/sp_utils.c b/src/sp_utils.c index a1fa400..3d2dfcc 100644 --- a/src/sp_utils.c +++ b/src/sp_utils.c @@ -19,7 +19,7 @@ const char* get_ipaddr() { return fwd_ip; } - return default_ipaddr; + return default_ipaddr; } void sp_log_msgf(char const* restrict feature, int level, int type, -- cgit v1.3 From 916a9d755a1660e086ef66d7113c2bcfc808a557 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Sat, 8 May 2021 16:14:49 +0200 Subject: Handle a possible issue with regexp Gracefully handle the case where we can't get allocated memory when trying to match a regex. --- src/sp_pcre_compat.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'src') diff --git a/src/sp_pcre_compat.c b/src/sp_pcre_compat.c index 509a8ea..0d19769 100644 --- a/src/sp_pcre_compat.c +++ b/src/sp_pcre_compat.c @@ -34,6 +34,9 @@ bool ZEND_HOT sp_is_regexp_matching_len(const sp_pcre* regexp, const char* str, #ifdef SP_HAS_PCRE2 pcre2_match_data* match_data = pcre2_match_data_create_from_pattern(regexp, NULL); + if (NULL == match_data) { + sp_log_err("regexp", "Unable to get memory for a regxp."); + } ret = pcre2_match(regexp, (PCRE2_SPTR)str, len, 0, 0, match_data, NULL); #else int vec[30]; -- cgit v1.3 From db14b8549e7bb9c132435f845a16f8d33677e865 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Sat, 8 May 2021 16:28:14 +0200 Subject: Fix a memory leak when using pcre2 --- src/sp_pcre_compat.c | 1 + 1 file changed, 1 insertion(+) (limited to 'src') diff --git a/src/sp_pcre_compat.c b/src/sp_pcre_compat.c index 0d19769..09a2fc7 100644 --- a/src/sp_pcre_compat.c +++ b/src/sp_pcre_compat.c @@ -38,6 +38,7 @@ bool ZEND_HOT sp_is_regexp_matching_len(const sp_pcre* regexp, const char* str, sp_log_err("regexp", "Unable to get memory for a regxp."); } ret = pcre2_match(regexp, (PCRE2_SPTR)str, len, 0, 0, match_data, NULL); + pcre2_match_data_free(match_data); #else int vec[30]; ret = pcre_exec(regexp, NULL, str, len, 0, 0, vec, sizeof(vec) / sizeof(int)); -- cgit v1.3 From ad623f1d78151dfe2eaee85e93ed1058be1c7f91 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Sat, 8 May 2021 16:45:07 +0200 Subject: Add a test for #390 --- src/tests/disable_function/config/disabled_functions.ini | 1 + .../disabled_functions_shell_exec_wrong.phpt | 14 ++++++++++++++ 2 files changed, 15 insertions(+) create mode 100644 src/tests/disable_function/disabled_functions_shell_exec_wrong.phpt (limited to 'src') diff --git a/src/tests/disable_function/config/disabled_functions.ini b/src/tests/disable_function/config/disabled_functions.ini index df7013f..0758c98 100644 --- a/src/tests/disable_function/config/disabled_functions.ini +++ b/src/tests/disable_function/config/disabled_functions.ini @@ -7,3 +7,4 @@ sp.disable_function.function_r("^var_dump$").drop(); sp.disable_function.function("sprintf").filename("/wrong file name").drop(); sp.disable_function.function("sprintf").filename("/wrong file name").drop(); sp.disable_function.function("eval").drop(); +sp.disable_function.function("shell_exec").param("foo").value("bar").drop(); diff --git a/src/tests/disable_function/disabled_functions_shell_exec_wrong.phpt b/src/tests/disable_function/disabled_functions_shell_exec_wrong.phpt new file mode 100644 index 0000000..580679c --- /dev/null +++ b/src/tests/disable_function/disabled_functions_shell_exec_wrong.phpt @@ -0,0 +1,14 @@ +--TEST-- +Disable functions - shell_exec, with a non-existing command +--SKIPIF-- + +--INI-- +sp.configuration_file={PWD}/config/disabled_functions.ini +--FILE-- + +--EXPECTF-- +%sfoo: not found +YES -- cgit v1.3 From 194b0bc9f0a4699854ea314ffa23e59f8082ddae Mon Sep 17 00:00:00 2001 From: jvoisin Date: Sat, 8 May 2021 17:14:46 +0200 Subject: Remove some memory-leaks --- src/snuffleupagus.c | 5 ++++- src/sp_config.c | 23 ++++++++++++++++++++++- src/sp_config.h | 1 + src/sp_pcre_compat.c | 5 +++++ src/sp_pcre_compat.h | 1 + 5 files changed, 33 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/snuffleupagus.c b/src/snuffleupagus.c index f192dd2..79a3003 100644 --- a/src/snuffleupagus.c +++ b/src/snuffleupagus.c @@ -154,8 +154,11 @@ PHP_MSHUTDOWN_FUNCTION(snuffleupagus) { FREE_LST_DISABLE(config_disabled_functions_reg_ret->disabled_functions); #undef FREE_LST_DISABLE + sp_list_node *_n = SNUFFLEUPAGUS_G(config).config_cookie->cookies; + sp_cookie_list_free(_n); + sp_list_free(_n); + #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); diff --git a/src/sp_config.c b/src/sp_config.c index 69730e3..958c7e5 100644 --- a/src/sp_config.c +++ b/src/sp_config.c @@ -216,11 +216,32 @@ void sp_disabled_function_list_free(sp_list_node *list) { sp_list_node *cursor = list; while (cursor) { sp_disabled_function *df = cursor->data; - if (df && df->functions_list) sp_list_free(df->functions_list); if (df) { + sp_list_free(df->functions_list); + sp_list_free(df->param_array_keys); + sp_list_free(df->var_array_keys); + + sp_pcre_free(df->r_filename); + sp_pcre_free(df->r_function); + sp_pcre_free(df->r_param); + sp_pcre_free(df->r_ret); + sp_pcre_free(df->r_value); + sp_pcre_free(df->r_key); + sp_tree_free(df->param); sp_tree_free(df->var); } cursor = cursor->next; } } + +void sp_cookie_list_free(sp_list_node *list) { + sp_list_node *cursor = list; + while (cursor) { + sp_cookie *c = cursor->data; + if (c) { + sp_pcre_free(c->name_r); + } + cursor = cursor->next; + } +} diff --git a/src/sp_config.h b/src/sp_config.h index b06e8be..e7b1473 100644 --- a/src/sp_config.h +++ b/src/sp_config.h @@ -282,5 +282,6 @@ int parse_list(char *restrict, char *restrict, void *); // cleanup void sp_disabled_function_list_free(sp_list_node *); +void sp_cookie_list_free(sp_list_node *); #endif /* SP_CONFIG_H */ diff --git a/src/sp_pcre_compat.c b/src/sp_pcre_compat.c index 09a2fc7..adcdee7 100644 --- a/src/sp_pcre_compat.c +++ b/src/sp_pcre_compat.c @@ -1,5 +1,10 @@ #include "php_snuffleupagus.h" +inline void sp_pcre_free(sp_pcre* regexp) { + pcre2_code_free(regexp); + regexp = NULL; +} + sp_pcre* sp_pcre_compile(const char* const pattern) { assert(NULL != pattern); diff --git a/src/sp_pcre_compat.h b/src/sp_pcre_compat.h index 14c33b2..725004d 100644 --- a/src/sp_pcre_compat.h +++ b/src/sp_pcre_compat.h @@ -17,6 +17,7 @@ #endif sp_pcre* sp_pcre_compile(const char* str); +void sp_pcre_free(sp_pcre* regexp); #define sp_is_regexp_matching_zend(regexp, zstr) \ sp_is_regexp_matching_len(regexp, ZSTR_VAL(zstr), ZSTR_LEN(zstr)) #define sp_is_regexp_matching(regexp, str) \ -- cgit v1.3 From 7ea172dd6909c624bd9db603c4e4f86e1042a7e5 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Sun, 9 May 2021 18:09:17 +0200 Subject: Fix compilation on non-pcre2 targets --- src/sp_pcre_compat.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src') diff --git a/src/sp_pcre_compat.c b/src/sp_pcre_compat.c index adcdee7..3bd00ca 100644 --- a/src/sp_pcre_compat.c +++ b/src/sp_pcre_compat.c @@ -1,7 +1,9 @@ #include "php_snuffleupagus.h" inline void sp_pcre_free(sp_pcre* regexp) { +#ifdef SP_HAS_PCRE2 pcre2_code_free(regexp); +#endif regexp = NULL; } -- cgit v1.3 From d5adcd6d17afc7015011088d8af5a2094fb3370d Mon Sep 17 00:00:00 2001 From: jvoisin Date: Sun, 9 May 2021 18:10:41 +0200 Subject: Fix the testsuite on fedora --- src/tests/disable_function/disabled_functions_shell_exec_wrong.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/tests/disable_function/disabled_functions_shell_exec_wrong.phpt b/src/tests/disable_function/disabled_functions_shell_exec_wrong.phpt index 580679c..fe8e73a 100644 --- a/src/tests/disable_function/disabled_functions_shell_exec_wrong.phpt +++ b/src/tests/disable_function/disabled_functions_shell_exec_wrong.phpt @@ -10,5 +10,5 @@ $gs = exec( 'foo' ); echo "YES"; ?> --EXPECTF-- -%sfoo: not found +%snot found YES -- cgit v1.3 From 8bd4d819012e4d426911c9f4a04e0f73bfe57888 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Sun, 9 May 2021 18:20:01 +0200 Subject: Allow session-related things to fail on php8 for now --- src/tests/session_encryption/crypt_session_corrupted_session.phpt | 1 + src/tests/session_encryption/crypt_session_invalid.phpt | 1 + 2 files changed, 2 insertions(+) (limited to 'src') diff --git a/src/tests/session_encryption/crypt_session_corrupted_session.phpt b/src/tests/session_encryption/crypt_session_corrupted_session.phpt index a89faf4..23f2580 100644 --- a/src/tests/session_encryption/crypt_session_corrupted_session.phpt +++ b/src/tests/session_encryption/crypt_session_corrupted_session.phpt @@ -2,6 +2,7 @@ Set a custom session handler --SKIPIF-- += 80000) print "skip"; ?> --INI-- sp.configuration_file={PWD}/config/config_crypt_session.ini session.save_path = "/tmp" diff --git a/src/tests/session_encryption/crypt_session_invalid.phpt b/src/tests/session_encryption/crypt_session_invalid.phpt index 9ec7c50..76fac5b 100644 --- a/src/tests/session_encryption/crypt_session_invalid.phpt +++ b/src/tests/session_encryption/crypt_session_invalid.phpt @@ -2,6 +2,7 @@ SESSION crypt and bad decrypt --SKIPIF-- += 80000) print "skip"; ?> --INI-- sp.configuration_file={PWD}/config/config_crypt_session.ini --ENV-- -- cgit v1.3 From 49d1664cd3708482c954ef4ffdddc54d3e7cbcf0 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Sun, 9 May 2021 18:26:33 +0200 Subject: Fix the testsuite on php7.4 --- src/tests/session_encryption/crypt_session_corrupted_session.phpt | 1 + src/tests/session_encryption/crypt_session_invalid.phpt | 1 + 2 files changed, 2 insertions(+) (limited to 'src') diff --git a/src/tests/session_encryption/crypt_session_corrupted_session.phpt b/src/tests/session_encryption/crypt_session_corrupted_session.phpt index 23f2580..a97dbca 100644 --- a/src/tests/session_encryption/crypt_session_corrupted_session.phpt +++ b/src/tests/session_encryption/crypt_session_corrupted_session.phpt @@ -3,6 +3,7 @@ Set a custom session handler --SKIPIF-- = 80000) print "skip"; ?> += 70400) print "skip"; ?> --INI-- sp.configuration_file={PWD}/config/config_crypt_session.ini session.save_path = "/tmp" diff --git a/src/tests/session_encryption/crypt_session_invalid.phpt b/src/tests/session_encryption/crypt_session_invalid.phpt index 76fac5b..967d9d1 100644 --- a/src/tests/session_encryption/crypt_session_invalid.phpt +++ b/src/tests/session_encryption/crypt_session_invalid.phpt @@ -3,6 +3,7 @@ SESSION crypt and bad decrypt --SKIPIF-- = 80000) print "skip"; ?> += 70400) print "skip"; ?> --INI-- sp.configuration_file={PWD}/config/config_crypt_session.ini --ENV-- -- cgit v1.3 From ec67149705739f9c13dc1f5dee335768cab3d7a0 Mon Sep 17 00:00:00 2001 From: WhiteWinterWolf Date: Sun, 9 May 2021 18:56:38 +0200 Subject: Fix disable function chmod --- config/default.rules | 5 +++-- config/default_php8.rules | 5 +++-- .../disable_function/config/disabled_functions_chmod.ini | 4 ++++ src/tests/disable_function/disabled_functions_chmod.phpt | 14 ++++++++++++++ .../disable_function/disabled_functions_chmod_php8.phpt | 14 ++++++++++++++ 5 files changed, 38 insertions(+), 4 deletions(-) create mode 100644 src/tests/disable_function/config/disabled_functions_chmod.ini create mode 100644 src/tests/disable_function/disabled_functions_chmod.phpt create mode 100644 src/tests/disable_function/disabled_functions_chmod_php8.phpt (limited to 'src') diff --git a/config/default.rules b/config/default.rules index 74e1edb..ea65e01 100644 --- a/config/default.rules +++ b/config/default.rules @@ -33,8 +33,9 @@ sp.disable_xxe.enable(); # https://snuffleupagus.readthedocs.io/features.html#protection-against-cross-site-request-forgery sp.cookie.name("PHPSESSID").samesite("lax"); -# Harden the `chmod` function -sp.disable_function.function("chmod").param("mode").value_r("^[0-9]{2}[67]$").drop(); +# Harden the `chmod` function (0777 (oct = 511, 0666 = 438) +sp.disable_function.function("chmod").param("mode").value("438").drop(); +sp.disable_function.function("chmod").param("mode").value("511").drop(); # Prevent various `mail`-related vulnerabilities sp.disable_function.function("mail").param("additional_parameters").value_r("\\-").drop(); diff --git a/config/default_php8.rules b/config/default_php8.rules index 893bfbc..c024176 100644 --- a/config/default_php8.rules +++ b/config/default_php8.rules @@ -34,8 +34,9 @@ sp.disable_xxe.enable(); # https://snuffleupagus.readthedocs.io/features.html#protection-against-cross-site-request-forgery sp.cookie.name("PHPSESSID").samesite("lax"); -# Harden the `chmod` function -sp.disable_function.function("chmod").param("permissions").value_r("^[0-9]{2}[67]$").drop(); +# Harden the `chmod` function (0777 (oct = 511, 0666 = 438) +sp.disable_function.function("chmod").param("permissions").value("438").drop(); +sp.disable_function.function("chmod").param("permissions").value("511").drop(); # Prevent various `mail`-related vulnerabilities sp.disable_function.function("mail").param("additional_parameters").value_r("\\-").drop(); diff --git a/src/tests/disable_function/config/disabled_functions_chmod.ini b/src/tests/disable_function/config/disabled_functions_chmod.ini new file mode 100644 index 0000000..e601900 --- /dev/null +++ b/src/tests/disable_function/config/disabled_functions_chmod.ini @@ -0,0 +1,4 @@ +# PHP7 and below +sp.disable_function.function("chmod").param("mode").value("511").drop(); +# PHP8 +sp.disable_function.function("chmod").param("permissions").value("511").drop(); diff --git a/src/tests/disable_function/disabled_functions_chmod.phpt b/src/tests/disable_function/disabled_functions_chmod.phpt new file mode 100644 index 0000000..28f948d --- /dev/null +++ b/src/tests/disable_function/disabled_functions_chmod.phpt @@ -0,0 +1,14 @@ +--TEST-- +Disable functions - chmod +--SKIPIF-- + += 80000) print "skip"; ?> +--INI-- +sp.configuration_file={PWD}/config/disabled_functions_chmod.ini +--FILE-- + +--XFAIL-- +--EXPECTF-- +Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'chmod', because its argument '$mode' content (511) matched a rule in %a/disabled_function_chmod.php on line %d diff --git a/src/tests/disable_function/disabled_functions_chmod_php8.phpt b/src/tests/disable_function/disabled_functions_chmod_php8.phpt new file mode 100644 index 0000000..71bb034 --- /dev/null +++ b/src/tests/disable_function/disabled_functions_chmod_php8.phpt @@ -0,0 +1,14 @@ +--TEST-- +Disable functions - chmod, in php8 +--SKIPIF-- + + +--INI-- +sp.configuration_file={PWD}/config/disabled_functions_chmod.ini +--FILE-- + +--XFAIL-- +--EXPECTF-- +Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'chmod', because its argument '$permissions' content (511) matched a rule in %a/disabled_function_chmod_php8.php on line %d -- cgit v1.3 From c3115fc26daebd0fa7135c202154272e42fbfcfd Mon Sep 17 00:00:00 2001 From: jvoisin Date: Sun, 9 May 2021 22:32:20 +0200 Subject: Add some checks to prevent recursion upon config reloading --- src/snuffleupagus.c | 2 +- src/sp_upload_validation.c | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/snuffleupagus.c b/src/snuffleupagus.c index 79a3003..f360a0c 100644 --- a/src/snuffleupagus.c +++ b/src/snuffleupagus.c @@ -325,7 +325,7 @@ static PHP_INI_MH(OnUpdateConfiguration) { zend_hash_str_find( SNUFFLEUPAGUS_G(config).config_disabled_functions_ret_hooked, "echo", sizeof("echo") - 1)) && - NULL == zend_write_default) { + NULL == zend_write_default && zend_write != hook_echo) { zend_write_default = zend_write; zend_write = hook_echo; } diff --git a/src/sp_upload_validation.c b/src/sp_upload_validation.c index f3ae311..cebab3e 100644 --- a/src/sp_upload_validation.c +++ b/src/sp_upload_validation.c @@ -103,6 +103,10 @@ int sp_rfc1867_callback(unsigned int event, void *event_data, void **extra) { #endif void hook_upload() { + if (php_rfc1867_callback == sp_rfc1867_callback) { + return; + } + if (NULL == sp_rfc1867_orig_callback) { sp_rfc1867_orig_callback = php_rfc1867_callback; php_rfc1867_callback = sp_rfc1867_callback; -- cgit v1.3 From d934db81cdcd64086c8867bb422bfa7e0d18bb38 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Sun, 9 May 2021 22:55:36 +0200 Subject: strtok/strtok_r is a thing from the past, don't use it. --- src/snuffleupagus.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/snuffleupagus.c b/src/snuffleupagus.c index f360a0c..f0737b4 100644 --- a/src/snuffleupagus.c +++ b/src/snuffleupagus.c @@ -250,14 +250,15 @@ static PHP_INI_MH(OnUpdateConfiguration) { return FAILURE; } - glob_t globbuf; - char *config_file; - char *rest = new_value->val; + char *str = new_value->val; - while ((config_file = strtok_r(rest, ",", &rest))) { - int ret = glob(config_file, GLOB_NOCHECK, NULL, &globbuf); + while (1) { + // We don't care about overwriting new_value->val + char *config_file = strsep(&str, ","); + if (config_file == NULL) break; - if (ret != 0) { + glob_t globbuf; + if (0 != glob(config_file, GLOB_NOCHECK, NULL, &globbuf)) { SNUFFLEUPAGUS_G(is_config_valid) = SP_CONFIG_INVALID; globfree(&globbuf); return FAILURE; -- cgit v1.3 From 8353de00398b13f6c94eeab6e2401d2e590543c6 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Sun, 9 May 2021 23:12:07 +0200 Subject: Add some guard to prevent hooking recursion This shouldn't be necessary, but better safe than sorry. --- src/sp_execute.c | 24 +++++++++++++++--------- src/sp_sloppy.c | 5 +++-- 2 files changed, 18 insertions(+), 11 deletions(-) (limited to 'src') diff --git a/src/sp_execute.c b/src/sp_execute.c index de83a2a..7d078b0 100644 --- a/src/sp_execute.c +++ b/src/sp_execute.c @@ -274,17 +274,23 @@ int hook_execute(void) { TSRMLS_FETCH(); if (NULL == orig_execute_ex && NULL == orig_zend_stream_open) { - /* zend_execute_ex is used for "user" function calls */ - orig_execute_ex = zend_execute_ex; - zend_execute_ex = sp_execute_ex; + if (zend_execute_ex != sp_execute_ex) { + /* zend_execute_ex is used for "user" function calls */ + orig_execute_ex = zend_execute_ex; + zend_execute_ex = sp_execute_ex; + } - /* zend_execute_internal is used for "builtin" functions calls */ - orig_zend_execute_internal = zend_execute_internal; - zend_execute_internal = sp_zend_execute_internal; + if (zend_execute_internal != sp_zend_execute_internal) { + /* zend_execute_internal is used for "builtin" functions calls */ + orig_zend_execute_internal = zend_execute_internal; + zend_execute_internal = sp_zend_execute_internal; + } - /* zend_stream_open_function is used for include-related stuff */ - orig_zend_stream_open = zend_stream_open_function; - zend_stream_open_function = sp_stream_open; + if (zend_stream_open_function != sp_stream_open) { + /* zend_stream_open_function is used for include-related stuff */ + orig_zend_stream_open = zend_stream_open_function; + zend_stream_open_function = sp_stream_open; + } } return SUCCESS; diff --git a/src/sp_sloppy.c b/src/sp_sloppy.c index f9ed718..ff2d644 100644 --- a/src/sp_sloppy.c +++ b/src/sp_sloppy.c @@ -99,12 +99,13 @@ PHP_FUNCTION(sp_array_keys) { void hook_sloppy() { TSRMLS_FETCH(); - if (NULL == orig_zend_compile_file) { + if (NULL == orig_zend_compile_file && zend_compile_file != sp_compile_file) { orig_zend_compile_file = zend_compile_file; zend_compile_file = sp_compile_file; } - if (NULL == orig_zend_compile_string) { + if (NULL == orig_zend_compile_string && + zend_compile_string != sp_compile_string) { orig_zend_compile_string = zend_compile_string; zend_compile_string = sp_compile_string; } -- cgit v1.3 From a27aa204d6b1e556dc30b0426f96d2c9244e75f8 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Thu, 22 Jul 2021 17:46:01 +0200 Subject: Sprinkle some const --- src/sp_var_parser.c | 2 +- src/sp_var_value.c | 26 +++++++++++++------------- src/sp_wrapper.c | 4 ++-- 3 files changed, 16 insertions(+), 16 deletions(-) (limited to 'src') diff --git a/src/sp_var_parser.c b/src/sp_var_parser.c index b066b84..bb5a5c0 100644 --- a/src/sp_var_parser.c +++ b/src/sp_var_parser.c @@ -20,7 +20,7 @@ static sp_list_node *parse_str_tokens(const char *str, return tokens_list; } -static bool is_var_name_valid(const char *name) { +static bool is_var_name_valid(const char *const name) { static sp_pcre *regexp_const = NULL; static sp_pcre *regexp_var = NULL; diff --git a/src/sp_var_value.c b/src/sp_var_value.c index e351446..fe37f99 100644 --- a/src/sp_var_value.c +++ b/src/sp_var_value.c @@ -1,7 +1,7 @@ #include "php_snuffleupagus.h" -static zval *get_param_var(zend_execute_data *ed, const char *var_name, - bool print) { +static zval *get_param_var(const zend_execute_data *const ed, + const char *const var_name, bool print) { unsigned int nb_param = ed->func->common.num_args; for (unsigned int i = 0; i < nb_param; i++) { @@ -21,7 +21,7 @@ static zval *get_param_var(zend_execute_data *ed, const char *var_name, return NULL; } -static zval *get_local_var(zend_execute_data *ed, const char *var_name) { +static zval *get_local_var(zend_execute_data *ed, const char *const var_name) { zend_execute_data *orig_execute_data = ed; zend_execute_data *current = ed; @@ -52,7 +52,7 @@ static zval *get_local_var(zend_execute_data *ed, const char *var_name) { return NULL; } -static zval *get_constant(const char *value) { +static zval *get_constant(const char *const value) { zend_string *name = zend_string_init(value, strlen(value), 0); zval *zvalue = zend_get_constant_ex(name, NULL, ZEND_FETCH_CLASS_SILENT); zend_string_release(name); @@ -90,8 +90,8 @@ static zval *get_var_value(zend_execute_data *ed, const char *var_name, } } -static void *get_entry_hashtable(const HashTable *ht, const char *entry, - size_t entry_len) { +static void *get_entry_hashtable(const HashTable *const ht, + const char *const entry, size_t entry_len) { zval *zvalue = zend_hash_str_find(ht, entry, entry_len); if (!zvalue) { @@ -109,8 +109,8 @@ static void *get_entry_hashtable(const HashTable *ht, const char *entry, return zvalue; } -static zval *get_array_value(zend_execute_data *ed, zval *zvalue, - const sp_tree *tree) { +static zval *get_array_value(zend_execute_data *ed, const zval *const zvalue, + const sp_tree *const tree) { zval *idx_value = sp_get_var_value(ed, tree->idx, false); if (!zvalue || !idx_value) { @@ -118,7 +118,7 @@ static zval *get_array_value(zend_execute_data *ed, zval *zvalue, } if (Z_TYPE_P(zvalue) == IS_ARRAY) { - const zend_string *idx = sp_zval_to_zend_string(idx_value); + const zend_string *const idx = sp_zval_to_zend_string(idx_value); return get_entry_hashtable(Z_ARRVAL_P(zvalue), ZSTR_VAL(idx), ZSTR_LEN(idx)); } @@ -128,10 +128,10 @@ static zval *get_array_value(zend_execute_data *ed, zval *zvalue, static zval *get_object_property(zend_execute_data *ed, zval *object, const char *property, bool is_param) { - char *class_name = object->value.obj->ce->name->val; + const char *const class_name = object->value.obj->ce->name->val; HashTable *array = Z_OBJPROP_P(object); zval *zvalue = NULL; - zval *property_val = get_var_value(ed, property, is_param); + const zval *property_val = get_var_value(ed, property, is_param); size_t len; if (property_val) { @@ -161,7 +161,7 @@ static zval *get_object_property(zend_execute_data *ed, zval *object, return zvalue; } -static zend_class_entry *get_class(const char *value) { +static zend_class_entry *get_class(const char *const value) { zend_string *name = zend_string_init(value, strlen(value), 0); zend_class_entry *ce = zend_lookup_class(name); zend_string_release(name); @@ -170,7 +170,7 @@ static zend_class_entry *get_class(const char *value) { static zval *get_unknown_type(const char *restrict value, zval *zvalue, zend_class_entry *ce, zend_execute_data *ed, - const sp_tree *tree, bool is_param) { + const sp_tree *const tree, bool is_param) { if (ce) { zvalue = get_entry_hashtable(&ce->constants_table, value, strlen(value)); ce = NULL; diff --git a/src/sp_wrapper.c b/src/sp_wrapper.c index 277f23a..7610114 100644 --- a/src/sp_wrapper.c +++ b/src/sp_wrapper.c @@ -1,6 +1,6 @@ #include "php_snuffleupagus.h" -static bool wrapper_is_whitelisted(const zend_string *zs) { +static bool wrapper_is_whitelisted(const zend_string *const zs) { const sp_list_node *list = SNUFFLEUPAGUS_G(config).config_wrapper->whitelist; if (!zs) { @@ -18,7 +18,7 @@ static bool wrapper_is_whitelisted(const zend_string *zs) { void sp_disable_wrapper() { HashTable *orig = php_stream_get_url_stream_wrappers_hash(); - HashTable *orig_complete = pemalloc(sizeof(*orig_complete), 1); + HashTable *orig_complete = pemalloc(sizeof(HashTable), 1); zval *zv; zend_string *zs; -- cgit v1.3 From 693041ba2ed4aa9bafaf0ae55fde02667799f385 Mon Sep 17 00:00:00 2001 From: WhiteWinterWolf Date: Sun, 25 Jul 2021 16:03:49 +0200 Subject: Replace an odd call to strtok_r(). --- src/sp_config.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/sp_config.c b/src/sp_config.c index 958c7e5..26476a2 100644 --- a/src/sp_config.c +++ b/src/sp_config.c @@ -73,7 +73,11 @@ int parse_list(char *restrict line, char *restrict keyword, void *list_ptr) { } tmp = ZSTR_VAL(value); - while ((token = strtok_r(tmp, ",", &tmp))) { + while (1) { + token = strsep(&tmp, ","); + if (token == NULL) { + break; + } *list = sp_list_insert(*list, zend_string_init(token, strlen(token), 1)); } -- cgit v1.3 From e62f226c3ed885808c832040872fc2d73ca46dac Mon Sep 17 00:00:00 2001 From: jvoisin Date: Wed, 28 Jul 2021 11:55:57 +0200 Subject: Sprinkle even more `const` --- src/snuffleupagus.c | 8 ++++---- src/sp_config.c | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/snuffleupagus.c b/src/snuffleupagus.c index f0737b4..7bf3649 100644 --- a/src/snuffleupagus.c +++ b/src/snuffleupagus.c @@ -21,7 +21,7 @@ ZEND_DLEXPORT int sp_zend_startup(zend_extension *extension) { } // LCOV_EXCL_END -static inline void sp_op_array_handler(zend_op_array *op) { +static inline void sp_op_array_handler(zend_op_array *const op) { // We need a filename, and strict mode not already enabled on this op if (NULL == op->filename || op->fn_flags & ZEND_ACC_STRICT_TYPES) { return; @@ -121,7 +121,7 @@ PHP_MINIT_FUNCTION(snuffleupagus) { return SUCCESS; } -static void free_disabled_functions_hashtable(HashTable *ht) { +static void free_disabled_functions_hashtable(HashTable *const ht) { void *ptr = NULL; ZEND_HASH_FOREACH_PTR(ht, ptr) { sp_list_free(ptr); } ZEND_HASH_FOREACH_END(); @@ -186,7 +186,7 @@ PHP_MSHUTDOWN_FUNCTION(snuffleupagus) { } PHP_RINIT_FUNCTION(snuffleupagus) { - const sp_config_wrapper *config_wrapper = + const sp_config_wrapper *const config_wrapper = SNUFFLEUPAGUS_G(config).config_wrapper; #if defined(COMPILE_DL_SNUFFLEUPAGUS) && defined(ZTS) ZEND_TSRMLS_CACHE_UPDATE(); @@ -254,7 +254,7 @@ static PHP_INI_MH(OnUpdateConfiguration) { while (1) { // We don't care about overwriting new_value->val - char *config_file = strsep(&str, ","); + const char *config_file = strsep(&str, ","); if (config_file == NULL) break; glob_t globbuf; diff --git a/src/sp_config.c b/src/sp_config.c index 26476a2..c12b435 100644 --- a/src/sp_config.c +++ b/src/sp_config.c @@ -6,7 +6,7 @@ size_t sp_line_no; -sp_config_tokens const sp_func[] = { +static sp_config_tokens const sp_func[] = { {.func = parse_unserialize, .token = SP_TOKEN_UNSERIALIZE_HMAC}, {.func = parse_random, .token = SP_TOKEN_HARDEN_RANDOM}, {.func = parse_log_media, .token = SP_TOKEN_LOG_MEDIA}, -- cgit v1.3