From e8d3cd9b26f0b4d660e424f2657f11bbc01eb171 Mon Sep 17 00:00:00 2001 From: Giovanni Date: Wed, 22 Jul 2020 09:28:42 +0200 Subject: refactoring sp_log_* (#340) Co-authored-by: Giovanni Dante Grazioli --- src/sp_execute.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'src/sp_execute.c') diff --git a/src/sp_execute.c b/src/sp_execute.c index 4eae874..73cc560 100644 --- a/src/sp_execute.c +++ b/src/sp_execute.c @@ -18,10 +18,10 @@ ZEND_COLD static inline void terminate_if_writable(const char *filename) { SP_TOKEN_READONLY_EXEC); } if (true == config_ro_exec->simulation) { - sp_log_msg("readonly_exec", SP_LOG_SIMULATION, + sp_log_simulation("readonly_exec", "Attempted execution of a writable file (%s).", filename); } else { - sp_log_msg("readonly_exec", SP_LOG_DROP, + sp_log_drop("readonly_exec", "Attempted execution of a writable file (%s).", filename); zend_bailout(); } @@ -79,14 +79,14 @@ is_in_eval_and_whitelisted(const zend_execute_data *execute_data) { SP_TOKEN_EVAL_WHITELIST); } if (config_eval->simulation) { - sp_log_msg( - "Eval_whitelist", SP_LOG_SIMULATION, + sp_log_simulation( + "Eval_whitelist", "The function '%s' isn't in the eval whitelist, logging its call.", ZSTR_VAL(current_function)); return; } else { - sp_log_msg( - "Eval_whitelist", SP_LOG_DROP, + sp_log_drop( + "Eval_whitelist", "The function '%s' isn't in the eval whitelist, dropping its call.", ZSTR_VAL(current_function)); } -- cgit v1.3 From a0d21a189cf04bb963dce93dcbd0bd9694584a0b Mon Sep 17 00:00:00 2001 From: jvoisin Date: Wed, 12 Aug 2020 08:48:59 +0000 Subject: Allow empty configuration (#342) This commit allows php to run (with a warning) if there is no specified snuffleupagus configuration, instead of refusing to start.--- src/php_snuffleupagus.h | 6 +++- src/snuffleupagus.c | 34 ++++++++++++++------ src/sp_crypt.c | 4 +-- src/sp_disabled_functions.c | 8 ++--- src/sp_execute.c | 5 +-- src/sp_upload_validation.c | 13 ++++---- src/sp_utils.c | 36 ++++++++++++---------- .../broken_conf_no_file_specified.phpt | 4 +-- src/tests/loading.phpt | 4 +-- 9 files changed, 69 insertions(+), 45 deletions(-) (limited to 'src/sp_execute.c') diff --git a/src/php_snuffleupagus.h b/src/php_snuffleupagus.h index 0849d36..6b0e210 100644 --- a/src/php_snuffleupagus.h +++ b/src/php_snuffleupagus.h @@ -62,6 +62,10 @@ typedef void (*zif_handler)(INTERNAL_FUNCTION_PARAMETERS); #define TSRMLS_C #endif +#define SP_CONFIG_VALID 1 +#define SP_CONFIG_INVALID 0 +#define SP_CONFIG_NONE -1 + #include "sp_pcre_compat.h" #include "sp_list.h" #include "sp_tree.h" @@ -101,7 +105,7 @@ extern zend_module_entry snuffleupagus_module_entry; ZEND_BEGIN_MODULE_GLOBALS(snuffleupagus) size_t in_eval; sp_config config; -bool is_config_valid; +int is_config_valid; // 1 = valid, 0 = invalid, -1 = none bool allow_broken_configuration; HashTable *disabled_functions_hook; HashTable *sp_internal_functions_hook; diff --git a/src/snuffleupagus.c b/src/snuffleupagus.c index d62069c..7c69150 100644 --- a/src/snuffleupagus.c +++ b/src/snuffleupagus.c @@ -68,6 +68,7 @@ ZEND_DLEXPORT zend_extension zend_extension_entry = { STANDARD_ZEND_EXTENSION_PROPERTIES}; PHP_GINIT_FUNCTION(snuffleupagus) { + snuffleupagus_globals->is_config_valid = SP_CONFIG_NONE; snuffleupagus_globals->in_eval = 0; #define SP_INIT_HT(F) snuffleupagus_globals->F = \ @@ -186,8 +187,12 @@ PHP_RINIT_FUNCTION(snuffleupagus) { ZEND_TSRMLS_CACHE_UPDATE(); #endif - if (!SNUFFLEUPAGUS_G(allow_broken_configuration) && !SNUFFLEUPAGUS_G(is_config_valid)) { - sp_log_err("config", "Invalid configuration file"); + if (!SNUFFLEUPAGUS_G(allow_broken_configuration)) { + 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"); + } } // We need to disable wrappers loaded by extensions loaded after SNUFFLEUPAGUS. @@ -209,12 +214,23 @@ PHP_RINIT_FUNCTION(snuffleupagus) { PHP_RSHUTDOWN_FUNCTION(snuffleupagus) { return SUCCESS; } PHP_MINFO_FUNCTION(snuffleupagus) { + const char *valid_config; + switch(SNUFFLEUPAGUS_G(is_config_valid)) { + case SP_CONFIG_VALID: + valid_config = "yes"; + break; + case SP_CONFIG_INVALID: + valid_config = "invalid"; + break; + case SP_CONFIG_NONE: + default: + valid_config = "no"; + } php_info_print_table_start(); - php_info_print_table_row(2, "snuffleupagus support", "enabled"); + 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", - (SNUFFLEUPAGUS_G(is_config_valid) == true) ? "yes" : "no"); + php_info_print_table_row( 2, "Valid config", valid_config); php_info_print_table_end(); DISPLAY_INI_ENTRIES(); } @@ -234,14 +250,14 @@ static PHP_INI_MH(OnUpdateConfiguration) { int ret = glob(config_file, GLOB_NOCHECK, NULL, &globbuf); if (ret != 0) { - SNUFFLEUPAGUS_G(is_config_valid) = false; + SNUFFLEUPAGUS_G(is_config_valid) = SP_CONFIG_INVALID; globfree(&globbuf); return FAILURE; } for (size_t i = 0; globbuf.gl_pathv[i]; i++) { if (sp_parse_config(globbuf.gl_pathv[i]) != SUCCESS) { - SNUFFLEUPAGUS_G(is_config_valid) = false; + SNUFFLEUPAGUS_G(is_config_valid) = SP_CONFIG_INVALID; globfree(&globbuf); return FAILURE; } @@ -249,7 +265,7 @@ static PHP_INI_MH(OnUpdateConfiguration) { globfree(&globbuf); } - SNUFFLEUPAGUS_G(is_config_valid) = true; + SNUFFLEUPAGUS_G(is_config_valid) = SP_CONFIG_VALID; if ((SNUFFLEUPAGUS_G(config).config_sloppy->enable)) { hook_sloppy(); diff --git a/src/sp_crypt.c b/src/sp_crypt.c index b353ebe..c57ac0b 100644 --- a/src/sp_crypt.c +++ b/src/sp_crypt.c @@ -108,8 +108,8 @@ int decrypt_zval(zval *pDest, bool simulation, zend_hash_key *hash_key) { return ZEND_HASH_APPLY_KEEP; } else { sp_log_warn("cookie_encryption", - "Something went wrong with the decryption of %s", - hash_key ? ZSTR_VAL(hash_key->key) : "the session"); + "Something went wrong with the decryption of %s", + hash_key ? ZSTR_VAL(hash_key->key) : "the session"); efree(backup); return ZEND_HASH_APPLY_REMOVE; } diff --git a/src/sp_disabled_functions.c b/src/sp_disabled_functions.c index a7136df..7be1c34 100644 --- a/src/sp_disabled_functions.c +++ b/src/sp_disabled_functions.c @@ -575,12 +575,12 @@ ZEND_FUNCTION(eval_blacklist_callback) { } if (config_eval->simulation) { sp_log_simulation("eval", - "A call to %s was tried in eval, in %s:%d, logging it.", - current_function_name, ZSTR_VAL(filename), line_number); + "A call to %s was tried in eval, in %s:%d, logging it.", + current_function_name, ZSTR_VAL(filename), line_number); } else { sp_log_drop("eval", - "A call to %s was tried in eval, in %s:%d, dropping it.", - current_function_name, ZSTR_VAL(filename), line_number); + "A call to %s was tried in eval, in %s:%d, dropping it.", + current_function_name, ZSTR_VAL(filename), line_number); } efree(filename); } diff --git a/src/sp_execute.c b/src/sp_execute.c index 73cc560..140e227 100644 --- a/src/sp_execute.c +++ b/src/sp_execute.c @@ -19,10 +19,11 @@ ZEND_COLD static inline void terminate_if_writable(const char *filename) { } if (true == config_ro_exec->simulation) { sp_log_simulation("readonly_exec", - "Attempted execution of a writable file (%s).", filename); + "Attempted execution of a writable file (%s).", + filename); } else { sp_log_drop("readonly_exec", - "Attempted execution of a writable file (%s).", filename); + "Attempted execution of a writable file (%s).", filename); zend_bailout(); } } else { diff --git a/src/sp_upload_validation.c b/src/sp_upload_validation.c index 4ee7bd7..f3ae311 100644 --- a/src/sp_upload_validation.c +++ b/src/sp_upload_validation.c @@ -13,10 +13,11 @@ int sp_rfc1867_callback(unsigned int event, void *event_data, void **extra); int sp_rfc1867_callback_win(unsigned int event, void *event_data, void **extra) { - sp_log_simulation("upload_validation", - "The upload validation doesn't work for now on Windows yet, " - "see https://github.com/jvoisin/snuffleupagus/issues/248 for " - "details."); + sp_log_simulation( + "upload_validation", + "The upload validation doesn't work for now on Windows yet, " + "see https://github.com/jvoisin/snuffleupagus/issues/248 for " + "details."); return SUCCESS; } @@ -91,8 +92,8 @@ int sp_rfc1867_callback(unsigned int event, void *event_data, void **extra) { char *uri = getenv("REQUEST_URI"); int sim = config_upload->simulation; sp_log_auto("upload_validation", sim, - "The upload of %s on %s was rejected.", - filename, uri ? uri : "?"); + "The upload of %s on %s was rejected.", filename, + uri ? uri : "?"); } } ZEND_HASH_FOREACH_END(); diff --git a/src/sp_utils.c b/src/sp_utils.c index 8032e0a..4c78ce5 100644 --- a/src/sp_utils.c +++ b/src/sp_utils.c @@ -41,7 +41,7 @@ const char* get_ipaddr() { } void sp_log_msgf(char const* restrict feature, int level, int type, - const char* restrict fmt, ...) { + const char* restrict fmt, ...) { char* msg; va_list args; @@ -51,7 +51,7 @@ void sp_log_msgf(char const* restrict feature, int level, int type, const char* client_ip = get_ipaddr(); const char* logtype = NULL; - switch(type) { + switch (type) { case SP_TYPE_SIMULATION: logtype = "simulation"; break; @@ -80,7 +80,8 @@ void sp_log_msgf(char const* restrict feature, int level, int type, } case SP_ZEND: default: - zend_error(level, "[snuffleupagus][%s][%s][%s] %s", client_ip, feature, logtype, msg); + zend_error(level, "[snuffleupagus][%s][%s][%s] %s", client_ip, feature, + logtype, msg); break; } } @@ -280,26 +281,27 @@ void sp_log_disable(const char* restrict path, const char* restrict arg_name, char_repr = zend_string_to_char(arg_value); } if (alias) { - sp_log_auto("disabled_function", sim, - "Aborted execution on call of the function '%s', " - "because its argument '%s' content (%s) matched the rule '%s'", - path, arg_name, char_repr ? char_repr : "?", ZSTR_VAL(alias)); + sp_log_auto( + "disabled_function", sim, + "Aborted execution on call of the function '%s', " + "because its argument '%s' content (%s) matched the rule '%s'", + path, arg_name, char_repr ? char_repr : "?", ZSTR_VAL(alias)); } else { sp_log_auto("disabled_function", sim, - "Aborted execution on call of the function '%s', " - "because its argument '%s' content (%s) matched a rule", - path, arg_name, char_repr ? char_repr : "?"); + "Aborted execution on call of the function '%s', " + "because its argument '%s' content (%s) matched a rule", + path, arg_name, char_repr ? char_repr : "?"); } efree(char_repr); } else { if (alias) { sp_log_auto("disabled_function", sim, - "Aborted execution on call of the function '%s', " - "because of the the rule '%s'", - path, ZSTR_VAL(alias)); + "Aborted execution on call of the function '%s', " + "because of the the rule '%s'", + path, ZSTR_VAL(alias)); } else { sp_log_auto("disabled_function", sim, - "Aborted execution on call of the function '%s'", path); + "Aborted execution on call of the function '%s'", path); } } } @@ -327,9 +329,9 @@ void sp_log_disable_ret(const char* restrict path, path, char_repr ? char_repr : "?", ZSTR_VAL(alias)); } else { sp_log_auto("disabled_function", sim, - "Aborted execution on return of the function '%s', " - "because the function returned '%s', which matched a rule", - path, char_repr ? char_repr : "?"); + "Aborted execution on return of the function '%s', " + "because the function returned '%s', which matched a rule", + path, char_repr ? char_repr : "?"); } efree(char_repr); } diff --git a/src/tests/broken_configuration/broken_conf_no_file_specified.phpt b/src/tests/broken_configuration/broken_conf_no_file_specified.phpt index 8b360d4..cb2d95f 100644 --- a/src/tests/broken_configuration/broken_conf_no_file_specified.phpt +++ b/src/tests/broken_configuration/broken_conf_no_file_specified.phpt @@ -6,5 +6,5 @@ Broken configuration - No configuration file specified --FILE-- --EXPECT-- -Fatal error: [snuffleupagus][0.0.0.0][config][log] Invalid configuration file in Unknown on line 0 -Could not startup. +Warning: [snuffleupagus][0.0.0.0][config][log] No configuration specificed via sp.configuration_file in Unknown on line 0 +1 diff --git a/src/tests/loading.phpt b/src/tests/loading.phpt index 761917a..2514ec5 100644 --- a/src/tests/loading.phpt +++ b/src/tests/loading.phpt @@ -7,5 +7,5 @@ Check for snuffleupagus presence echo "snuffleupagus extension is available"; ?> --EXPECT-- -Fatal error: [snuffleupagus][0.0.0.0][config][log] Invalid configuration file in Unknown on line 0 -Could not startup. +Warning: [snuffleupagus][0.0.0.0][config][log] No configuration specificed via sp.configuration_file in Unknown on line 0 +snuffleupagus extension is available -- cgit v1.3 From a59c3622848840f55a22ed91bf782775f43a40a1 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Sun, 16 Aug 2020 13:14:46 +0200 Subject: Remove a useless line of code --- src/sp_execute.c | 1 - 1 file changed, 1 deletion(-) (limited to 'src/sp_execute.c') diff --git a/src/sp_execute.c b/src/sp_execute.c index 140e227..ab73972 100644 --- a/src/sp_execute.c +++ b/src/sp_execute.c @@ -24,7 +24,6 @@ ZEND_COLD static inline void terminate_if_writable(const char *filename) { } else { sp_log_drop("readonly_exec", "Attempted execution of a writable file (%s).", filename); - zend_bailout(); } } else { if (EACCES != errno) { -- cgit v1.3 From 56376c73c6896ea581b10d5041822eafeb6010c1 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Thu, 12 Nov 2020 19:21:37 +0100 Subject: Simplify a bit a function --- src/sp_execute.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) (limited to 'src/sp_execute.c') diff --git a/src/sp_execute.c b/src/sp_execute.c index ab73972..de83a2a 100644 --- a/src/sp_execute.c +++ b/src/sp_execute.c @@ -156,6 +156,7 @@ static void sp_execute_ex(zend_execute_data *execute_data) { return; } + // If we're at an internal function if (!execute_data->prev_execute_data || !execute_data->prev_execute_data->func || !ZEND_USER_CODE(execute_data->prev_execute_data->func->type) || @@ -163,17 +164,18 @@ static void sp_execute_ex(zend_execute_data *execute_data) { should_disable_ht(execute_data, function_name, NULL, NULL, config_disabled_functions_reg, config_disabled_functions); - } else if ((execute_data->prev_execute_data->opline->opcode == - ZEND_DO_FCALL || - execute_data->prev_execute_data->opline->opcode == - ZEND_DO_UCALL || - execute_data->prev_execute_data->opline->opcode == - ZEND_DO_ICALL || - execute_data->prev_execute_data->opline->opcode == - ZEND_DO_FCALL_BY_NAME)) { - should_disable_ht(execute_data, function_name, NULL, NULL, - config_disabled_functions_reg, - config_disabled_functions); + } else { // If we're at a userland function call + switch (execute_data->prev_execute_data->opline->opcode) { + case ZEND_DO_FCALL: + case ZEND_DO_FCALL_BY_NAME: + case ZEND_DO_ICALL: + case ZEND_DO_UCALL: + should_disable_ht(execute_data, function_name, NULL, NULL, + config_disabled_functions_reg, + config_disabled_functions); + default: + break; + } } // When a function's return value isn't used, php doesn't store it in the -- cgit v1.3