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.

8 files changed, 75 insertions, 11 deletions

diff --git a/src/php_snuffleupagus.h b/src/php_snuffleupagus.h
index 53cf1da..6e035d4 100644
--- a/src/php_snuffleupagus.h
+++ b/src/php_snuffleupagus.h
@@ -17,6 +17,7 @@
 #include <glob.h>
 #endif
 
+#include <ctype.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <inttypes.h>
diff --git a/src/sp_ini.c b/src/sp_ini.c
index 7b22012..8860a92 100644
--- a/src/sp_ini.c
+++ b/src/sp_ini.c
@@ -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) {
@@ -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;
     }
@@ -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;
   }
@@ -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
@@ -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;
     }
   }
diff --git a/src/tests/ini/ini_min_policy_drop.phpt b/src/tests/ini/ini_min_policy_drop.phpt
index 1ec9f9a..43e180e 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 facb73e..10c15a4 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 dfc2555..0835222 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 c7cab35..3d2156c 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 432be8d..134e5c3 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
diff --git a/src/tests/ini/ini_regexp_drop_base64.phpt b/src/tests/ini/ini_regexp_drop_base64.phpt
new file mode 100644
index 0000000..32076d5
--- /dev/null
+++ b/src/tests/ini/ini_regexp_drop_base64.phpt
@@ -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