From dbee25739c3e7d6a6288dec95c58e9f96fa887d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Wed, 13 Dec 2023 20:37:42 +0100 Subject: [PATCH] print key and value on INI violations On violations of INI settings include the key and if appropriate the value in the log message. This helps to locate offenders and fine tune the configuration itself. --- src/sp_ini.c | 65 ++++++++++++++++++++++++-- src/tests/ini/ini_min_policy_drop.phpt | 2 +- src/tests/ini/ini_minmax.phpt | 4 +- src/tests/ini/ini_null.phpt | 2 +- src/tests/ini/ini_regexp.phpt | 2 +- src/tests/ini/ini_regexp_drop.phpt | 2 +- 6 files changed, 66 insertions(+), 11 deletions(-) diff --git a/src/sp_ini.c b/src/sp_ini.c index 7b22012a..bac75a04 100644 --- a/src/sp_ini.c +++ b/src/sp_ini.c @@ -11,6 +11,33 @@ sp_log_auto2("ini_protection", simulation, (cfg->policy_drop || (entry && entry->drop)), __VA_ARGS__); \ } +static const char* check_safe_key(const char* string) { + for (const char* p = string; *p != '\0'; p++) { + if (isalnum((unsigned char)*p) || *p == '_' || *p == '.') { + continue; + } + + return NULL; + } + + return string; +} + +static const char* check_safe_value(const char* string) { + for (const char* p = string; *p != '\0'; p++) { + if (*p == '\'' || *p == '"') { + return NULL; + } + + if (isgraph((unsigned char)*p)) { + continue; + } + + return NULL; + } + + return string; +} static bool /* success */ sp_ini_check(zend_string *const restrict varname, zend_string const *const restrict new_value, sp_ini_entry **sp_entry_p) { if (!varname || ZSTR_LEN(varname) == 0) { @@ -27,7 +54,7 @@ static bool /* success */ sp_ini_check(zend_string *const restrict varname, zend if (!entry) { if (cfg->policy_readonly) { if (!cfg->policy_silent_ro) { - sp_log_ini_check_violation("INI setting is read-only"); + sp_log_ini_check_violation("INI setting `%s` is read-only", check_safe_key(ZSTR_VAL(varname)) ?: ""); } return simulation; } @@ -38,7 +65,11 @@ static bool /* success */ sp_ini_check(zend_string *const restrict varname, zend if (SP_INI_ACCESS_READONLY_COND(entry, cfg)) { if (!cfg->policy_silent_ro) { - sp_log_ini_check_violation("%s", (entry->msg ? ZSTR_VAL(entry->msg) : "INI setting is read-only")); + if (entry->msg) { + sp_log_ini_check_violation("%s", ZSTR_VAL(entry->msg)); + } else { + sp_log_ini_check_violation("INI setting `%s` is read-only", ZSTR_VAL(entry->key)); + } } return simulation; } @@ -48,7 +79,7 @@ static bool /* success */ sp_ini_check(zend_string *const restrict varname, zend return true; // allow NULL value and skip other tests } if (SP_INI_HAS_CHECKS_COND(entry)) { - sp_log_ini_check_violation("new INI value must not be NULL or empty"); + sp_log_ini_check_violation("new INI value for `%s` must not be NULL or empty", ZSTR_VAL(entry->key)); return simulation; } return true; // no new_value, but no checks to perform @@ -66,14 +97,38 @@ static bool /* success */ sp_ini_check(zend_string *const restrict varname, zend if ((entry->min && zend_atol(ZSTR_VAL(entry->min), ZSTR_LEN(entry->min)) > lvalue) || (entry->max && zend_atol(ZSTR_VAL(entry->max), ZSTR_LEN(entry->max)) < lvalue)) { #endif - sp_log_ini_check_violation("%s", (entry->msg ? ZSTR_VAL(entry->msg) : "INI value out of range")); + if (entry->msg) { + sp_log_ini_check_violation("%s", ZSTR_VAL(entry->msg)); + } else { + sp_log_ini_check_violation("INI value %lld for `%s` out of range", lvalue, ZSTR_VAL(entry->key)); + } return simulation; } } if (entry->regexp) { if (!sp_is_regexp_matching_zstr(entry->regexp, new_value)) { - sp_log_ini_check_violation("%s", (entry->msg ? ZSTR_VAL(entry->msg) : "INI value does not match regex")); + if (entry->msg) { + sp_log_ini_check_violation("%s", ZSTR_VAL(entry->msg)); + } else { + zend_string *base64_new_val = NULL; + const char *safe_new_val; + + safe_new_val = check_safe_value(ZSTR_VAL(new_value)); + if (!safe_new_val) { + base64_new_val = php_base64_encode_str(new_value); + safe_new_val = base64_new_val ? ZSTR_VAL(base64_new_val) : ""; + } + + sp_log_ini_check_violation("INI value `%s`%s for `%s` does not match regex", + safe_new_val, + base64_new_val ? "(base64)" : "", + ZSTR_VAL(entry->key)); + + if (base64_new_val) { + zend_string_release(base64_new_val); + } + } return simulation; } } diff --git a/src/tests/ini/ini_min_policy_drop.phpt b/src/tests/ini/ini_min_policy_drop.phpt index 1ec9f9a7..43e180e9 100644 --- a/src/tests/ini/ini_min_policy_drop.phpt +++ b/src/tests/ini/ini_min_policy_drop.phpt @@ -10,4 +10,4 @@ var_dump(ini_set("max_execution_time", "29") === false); var_dump(ini_get("max_execution_time")); ?> --EXPECTF-- -Fatal error: [snuffleupagus][0.0.0.0][ini_protection][drop] INI value out of range in %a/ini_min_policy_drop.php on line 2%A +Fatal error: [snuffleupagus][0.0.0.0][ini_protection][drop] INI value 29 for `max_execution_time` out of range in %a/ini_min_policy_drop.php on line 2%A diff --git a/src/tests/ini/ini_minmax.phpt b/src/tests/ini/ini_minmax.phpt index facb73e7..10c15a46 100644 --- a/src/tests/ini/ini_minmax.phpt +++ b/src/tests/ini/ini_minmax.phpt @@ -25,10 +25,10 @@ string(2) "30" bool(false) string(3) "300" -Warning: [snuffleupagus][0.0.0.0][ini_protection][log] INI value out of range in %a/ini_minmax.php on line 8 +Warning: [snuffleupagus][0.0.0.0][ini_protection][log] INI value 29 for `max_execution_time` out of range in %a/ini_minmax.php on line 8 bool(true) string(3) "300" -Warning: [snuffleupagus][0.0.0.0][ini_protection][log] INI value out of range in %a/ini_minmax.php on line 11 +Warning: [snuffleupagus][0.0.0.0][ini_protection][log] INI value 301 for `max_execution_time` out of range in %a/ini_minmax.php on line 11 bool(true) string(3) "300"%A diff --git a/src/tests/ini/ini_null.phpt b/src/tests/ini/ini_null.phpt index dfc25555..08352225 100644 --- a/src/tests/ini/ini_null.phpt +++ b/src/tests/ini/ini_null.phpt @@ -21,6 +21,6 @@ string(15) "foo@example.com" bool(false) string(0) "" -Warning: [snuffleupagus][0.0.0.0][ini_protection][log] new INI value must not be NULL or empty in %a/ini_null.php on line 8 +Warning: [snuffleupagus][0.0.0.0][ini_protection][log] new INI value for `unserialize_callback_func` must not be NULL or empty in %a/ini_null.php on line 8 bool(true) string(3) "def"%A diff --git a/src/tests/ini/ini_regexp.phpt b/src/tests/ini/ini_regexp.phpt index c7cab353..3d2156cf 100644 --- a/src/tests/ini/ini_regexp.phpt +++ b/src/tests/ini/ini_regexp.phpt @@ -15,5 +15,5 @@ var_dump(ini_get("highlight.comment")); --EXPECTF-- string(7) "#000aBc" -Warning: [snuffleupagus][0.0.0.0][ini_protection][log] INI value does not match regex in %a/ini_regexp.php on line 5 +Warning: [snuffleupagus][0.0.0.0][ini_protection][log] INI value `xxx` for `highlight.comment` does not match regex in %a/ini_regexp.php on line 5 string(7) "#000aBc"%A diff --git a/src/tests/ini/ini_regexp_drop.phpt b/src/tests/ini/ini_regexp_drop.phpt index 432be8df..134e5c32 100644 --- a/src/tests/ini/ini_regexp_drop.phpt +++ b/src/tests/ini/ini_regexp_drop.phpt @@ -10,4 +10,4 @@ var_dump(ini_set("user_agent", "Foo") === false); var_dump(ini_get("user_agent")); ?> --EXPECTF-- -Fatal error: [snuffleupagus][0.0.0.0][ini_protection][drop] INI value does not match regex in %a/ini_regexp_drop.php on line 2%A%A%A%A +Fatal error: [snuffleupagus][0.0.0.0][ini_protection][drop] INI value `Foo` for `user_agent` does not match regex in %a/ini_regexp_drop.php on line 2%A%A%A%A