Skip to content

Commit

Permalink
print key and value on INI violations
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
cgzones authored and jvoisin committed Dec 13, 2023
1 parent fed8c8f commit 3c720be
Show file tree
Hide file tree
Showing 8 changed files with 75 additions and 11 deletions.
1 change: 1 addition & 0 deletions src/php_snuffleupagus.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <glob.h>
#endif

#include <ctype.h>
#include <errno.h>
#include <fcntl.h>
#include <inttypes.h>
Expand Down
60 changes: 55 additions & 5 deletions src/sp_ini.c
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,29 @@
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 != '.') {
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)) {
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) {
Expand All @@ -27,7 +50,8 @@ 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)) ? ZSTR_VAL(varname) : "<invalid>");
}
return simulation;
}
Expand All @@ -38,7 +62,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;
}
Expand All @@ -48,7 +76,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
Expand All @@ -66,14 +94,36 @@ 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 = check_safe_value(ZSTR_VAL(new_value));
if (!safe_new_val) {
base64_new_val = php_base64_encode((const unsigned char*)(ZSTR_VAL(new_value)), ZSTR_LEN(new_value));
safe_new_val = base64_new_val ? ZSTR_VAL(base64_new_val) : "<invalid>";
}

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;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/tests/ini/ini_min_policy_drop.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 2 additions & 2 deletions src/tests/ini/ini_minmax.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion src/tests/ini/ini_null.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,6 @@ string(15) "[email protected]"
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
2 changes: 1 addition & 1 deletion src/tests/ini/ini_regexp.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion src/tests/ini/ini_regexp_drop.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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
13 changes: 13 additions & 0 deletions src/tests/ini/ini_regexp_drop_base64.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
--TEST--
INI protection .min() + .drop(), log base64
--SKIPIF--
<?php if (!extension_loaded("snuffleupagus")) print("skip"); ?>
--INI--
sp.configuration_file={PWD}/config/sp.ini
--FILE--
<?php
var_dump(ini_set("user_agent", "Foo\n\r") === false);
var_dump(ini_get("user_agent"));
?>
--EXPECTF--
Fatal error: [snuffleupagus][0.0.0.0][ini_protection][drop] INI value `Rm9vCg0=`(base64) for `user_agent` does not match regex in %a/ini_regexp_drop_base64.php on line 2%A%A%A%A

0 comments on commit 3c720be

Please sign in to comment.