From 2dcf2a2d7578d1e43ee7e3fa69386ccc5afebbf0 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Wed, 1 Feb 2023 21:12:58 +0100 Subject: Url encode functions arguments when logging them --- doc/source/config.rst | 3 +++ src/sp_utils.c | 24 +++++++++------------- .../disabled_functions_eval_param.phpt | 2 +- .../disabled_functions_include_once.phpt | 2 +- .../disabled_functions_include_simulation.phpt | 2 +- .../disabled_functions_nul_byte.phpt | 2 +- .../disabled_functions_require.phpt | 2 +- .../disabled_functions_require_once.phpt | 2 +- .../disabled_functions_require_simulation.phpt | 2 +- 9 files changed, 20 insertions(+), 21 deletions(-) diff --git a/doc/source/config.rst b/doc/source/config.rst index 2c5fc96..40e8c11 100644 --- a/doc/source/config.rst +++ b/doc/source/config.rst @@ -497,6 +497,9 @@ not the one where the function is **called from**. For clarity, the presence of the ``allow`` or ``drop`` action is **mandatory**. +In the logs, the parameters and the return values of function are url-encoded, +to accommodate fragile log processors. + .. warning:: When you're writing rules, please do keep in mind that **the order matters**. diff --git a/src/sp_utils.c b/src/sp_utils.c index 1bac1ae..eeebcc4 100644 --- a/src/sp_utils.c +++ b/src/sp_utils.c @@ -232,16 +232,6 @@ static char* zend_string_to_char(const zend_string* zs) { return copy; } -static void sp_sanitize_charstring(char* c, size_t maxlen) -{ - for (size_t i = 0; i < maxlen - 1; i++) { - if (c[i] < 32 || c[i] > 126) { - c[i] = '*'; - } - } - c[maxlen] = 0; -} - const zend_string* sp_zval_to_zend_string(const zval* zv) { switch (Z_TYPE_P(zv)) { case IS_LONG: { @@ -300,8 +290,11 @@ void sp_log_disable(const char* restrict path, const char* restrict arg_name, if (arg_name) { char* char_repr = NULL; if (arg_value) { - char_repr = zend_string_to_char(arg_value); - sp_sanitize_charstring(char_repr, MIN(ZSTR_LEN(arg_value), (size_t)SPCFG(log_max_len))); + zend_string *arg_value_dup = zend_string_init(ZSTR_VAL(arg_value), ZSTR_LEN(arg_value), 0); + arg_value_dup = php_raw_url_encode(ZSTR_VAL(arg_value_dup), ZSTR_LEN(arg_value_dup)); + char_repr = zend_string_to_char(arg_value_dup); + size_t max_len = MIN(ZSTR_LEN(arg_value_dup), (size_t)SPCFG(log_max_len)); + char_repr[max_len] = '\0'; } if (alias) { sp_log_auto( @@ -341,8 +334,11 @@ void sp_log_disable_ret(const char* restrict path, sp_log_request(dump, config_node->textual_representation); } if (ret_value) { - char_repr = zend_string_to_char(ret_value); - sp_sanitize_charstring(char_repr, MIN(ZSTR_LEN(ret_value), (size_t)SPCFG(log_max_len))); + zend_string *ret_value_dup = zend_string_init(ZSTR_VAL(ret_value), ZSTR_LEN(ret_value), 0); + ret_value_dup = php_raw_url_encode(ZSTR_VAL(ret_value_dup), ZSTR_LEN(ret_value_dup)); + char_repr = zend_string_to_char(ret_value_dup); + size_t max_len = MIN(ZSTR_LEN(ret_value_dup), (size_t)SPCFG(log_max_len)); + char_repr[max_len] = '\0'; } if (alias) { sp_log_auto( diff --git a/src/tests/disable_function/disabled_functions_eval_param.phpt b/src/tests/disable_function/disabled_functions_eval_param.phpt index 4f3f1ef..7d0487a 100644 --- a/src/tests/disable_function/disabled_functions_eval_param.phpt +++ b/src/tests/disable_function/disabled_functions_eval_param.phpt @@ -11,4 +11,4 @@ eval('$var = 1337 + 1337;'); print("Variable: $var\n"); ?> --EXPECTF-- -Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'eval', because its argument 'code' content ($var = 1337 + 1337;) matched a rule in %s/tests/disable_function/disabled_functions_eval_param.php(3) : eval()'d code on line 1 +Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'eval', because its argument 'code' content (%24var%20%3D%201337%20%2B%201337%3B) matched a rule in %s/tests/disable_function/disabled_functions_eval_param.php(3) : eval()'d code on line 1 diff --git a/src/tests/disable_function/disabled_functions_include_once.phpt b/src/tests/disable_function/disabled_functions_include_once.phpt index 8b1bec8..91d9497 100644 --- a/src/tests/disable_function/disabled_functions_include_once.phpt +++ b/src/tests/disable_function/disabled_functions_include_once.phpt @@ -21,6 +21,6 @@ echo "1337\n"; --EXPECTF-- BLA -Warning: [snuffleupagus][0.0.0.0][disabled_function][simulation] Aborted execution on call of the function 'include_once', because its argument 'inclusion path' content (%a/test.sim) matched a rule in %a/disabled_functions_include_once.php on line %d +Warning: [snuffleupagus][0.0.0.0][disabled_function][simulation] Aborted execution on call of the function 'include_once', because its argument 'inclusion path' content (%a%2Ftest.sim) matched a rule in %a/disabled_functions_include_once.php on line %d MEH 1337 diff --git a/src/tests/disable_function/disabled_functions_include_simulation.phpt b/src/tests/disable_function/disabled_functions_include_simulation.phpt index cf2c693..c2bd48b 100644 --- a/src/tests/disable_function/disabled_functions_include_simulation.phpt +++ b/src/tests/disable_function/disabled_functions_include_simulation.phpt @@ -21,6 +21,6 @@ echo "1337\n"; --EXPECTF-- BLA -Warning: [snuffleupagus][0.0.0.0][disabled_function][simulation] Aborted execution on call of the function 'include', because its argument 'inclusion path' content (%a/test.sim) matched a rule in %a/disabled_functions_include_simulation.php on line %d +Warning: [snuffleupagus][0.0.0.0][disabled_function][simulation] Aborted execution on call of the function 'include', because its argument 'inclusion path' content (%a%2Ftest.sim) matched a rule in %a/disabled_functions_include_simulation.php on line %d MEH 1337 diff --git a/src/tests/disable_function/disabled_functions_nul_byte.phpt b/src/tests/disable_function/disabled_functions_nul_byte.phpt index 62f4ab5..991794d 100644 --- a/src/tests/disable_function/disabled_functions_nul_byte.phpt +++ b/src/tests/disable_function/disabled_functions_nul_byte.phpt @@ -11,4 +11,4 @@ system("id"); ?> --EXPECTF-- -Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'system', because its argument '$command' content (0id) matched a rule in %a/disabled_functions_nul_byte.php on line 2 +Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'system', because its argument '$command' content (%s0id) matched a rule in %a/disabled_functions_nul_byte.php on line 2 diff --git a/src/tests/disable_function/disabled_functions_require.phpt b/src/tests/disable_function/disabled_functions_require.phpt index bf59b58..a759a33 100644 --- a/src/tests/disable_function/disabled_functions_require.phpt +++ b/src/tests/disable_function/disabled_functions_require.phpt @@ -20,4 +20,4 @@ echo "1337"; ?> --EXPECTF-- BLA -Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'require', because its argument 'inclusion path' content (%a/test.meh) matched a rule in %a/disabled_functions_require.php on line %d +Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'require', because its argument 'inclusion path' content (%a%2Ftest.meh) matched a rule in %a/disabled_functions_require.php on line %d diff --git a/src/tests/disable_function/disabled_functions_require_once.phpt b/src/tests/disable_function/disabled_functions_require_once.phpt index 81049ef..62b8d4c 100644 --- a/src/tests/disable_function/disabled_functions_require_once.phpt +++ b/src/tests/disable_function/disabled_functions_require_once.phpt @@ -19,4 +19,4 @@ echo "1337"; ?> --EXPECTF-- BLA -Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'require_once', because its argument 'inclusion path' content (%a/test.meh) matched a rule in %a/disabled_functions_require_once.php on line %d +Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'require_once', because its argument 'inclusion path' content (%a%2Ftest.meh) matched a rule in %a/disabled_functions_require_once.php on line %d diff --git a/src/tests/disable_function/disabled_functions_require_simulation.phpt b/src/tests/disable_function/disabled_functions_require_simulation.phpt index 2c52610..d23ad4e 100644 --- a/src/tests/disable_function/disabled_functions_require_simulation.phpt +++ b/src/tests/disable_function/disabled_functions_require_simulation.phpt @@ -20,6 +20,6 @@ echo "1337\n"; --EXPECTF-- BLA -Warning: [snuffleupagus][0.0.0.0][disabled_function][simulation] Aborted execution on call of the function 'require', because its argument 'inclusion path' content (%a/test.sim) matched a rule in %a/disabled_functions_require_simulation.php on line %d +Warning: [snuffleupagus][0.0.0.0][disabled_function][simulation] Aborted execution on call of the function 'require', because its argument 'inclusion path' content (%a%2Ftest.sim) matched a rule in %a/disabled_functions_require_simulation.php on line %d MEH 1337 -- cgit v1.3