13 files changed, 125 insertions, 30 deletions

diff --git a/config/default.rules b/config/default.rules
index 3e82ae3..0fa4878 100644
--- a/config/default.rules
+++ b/config/default.rules
@@ -35,6 +35,10 @@ sp.xxe_protection.enable();
 # https://snuffleupagus.readthedocs.io/features.html#protection-against-cross-site-request-forgery
 sp.cookie.name("PHPSESSID").samesite("lax");
 
+# Note that an attacker with arbitrary PHP code execution
+# can bypass some virtual-patching, by (as)using PHP feature.
+# A clever example would be to declare a class with a __toString method.
+
 # Harden the `chmod` function (0777 (oct = 511, 0666 = 438)
 @condition PHP_VERSION_ID < 80000;
     sp.disable_function.function("chmod").param("mode").value("438").drop();
diff --git a/doc/source/config.rst b/doc/source/config.rst
index 2053c2f..a84bb60 100644
--- a/doc/source/config.rst
+++ b/doc/source/config.rst
@@ -152,7 +152,7 @@ least astonishment
 <https://en.wikipedia.org/wiki/Principle_of_least_astonishment>`__. But since
 it's `possible to modify php's logging system via php
 <https://www.php.net/manual/en/errorfunc.configuration.php>`__, it's
-heavily recommended to use the ``syslog`` option instead. The ``file:` option
+heavily recommended to use the ``syslog`` option instead. The ``file:`` option
 might be useful if you're using Snuffleupagus to fuzz or audit a codebase.
 
 log_max_len
diff --git a/doc/source/features.rst b/doc/source/features.rst
index adb8779..517bbec 100644
--- a/doc/source/features.rst
+++ b/doc/source/features.rst
@@ -309,7 +309,11 @@ of dangerous functions, dropping them everywhere else:
    :language: php
 
 
-The intent is to make post-exploitation process (such as backdooring of legitimate code, or RAT usage) a lot harder for the attacker.
+The intent is to make post-exploitation process (such as backdooring of
+legitimate code, or RAT usage) a lot harder for the attacker.
+
+Note that an attacker able to run arbitrary PHP code can likely bypass some virtual-patching
+by (ab)using some PHP features.
 
 
 .. _global-strict-feature:
diff --git a/src/snuffleupagus.c b/src/snuffleupagus.c
index 4d5fa09..6b0a327 100644
--- a/src/snuffleupagus.c
+++ b/src/snuffleupagus.c
@@ -25,6 +25,39 @@ static inline void sp_op_array_handler(zend_op_array *const op) {
       op->fn_flags |= ZEND_ACC_STRICT_TYPES;
     }
   }
+#if PHP_VERSION_ID >= 80500
+  /* Prevent opcache from inlining user functions that have return-value
+   * monitoring rules, otherwise zend_execute_ex is never called and the
+   * hook never fires. ZEND_ACC_HAS_TYPE_HINTS is checked by
+   * zend_try_inline_call() and blocks inlining. For functions without
+   * actual type hints the only runtime effect is that ZEND_RECV opcodes
+   * are executed instead of skipped; for 0-arg functions (the common
+   * inlineable case) there are no RECV opcodes so the impact is zero. */
+  if (op->function_name && ZEND_USER_CODE(op->type) &&
+      ((SPCFG(disabled_functions_ret) && zend_hash_num_elements(SPCFG(disabled_functions_ret))) ||
+       SPCFG(disabled_functions_reg_ret).disabled_functions)) {
+    char *fname = NULL;
+    if (op->scope) {
+      const size_t len = ZSTR_LEN(op->scope->name) + 2 + ZSTR_LEN(op->function_name) + 1;
+      fname = emalloc(len);
+      snprintf(fname, len, "%s::%s", ZSTR_VAL(op->scope->name), ZSTR_VAL(op->function_name));
+    } else {
+      fname = estrdup(ZSTR_VAL(op->function_name));
+    }
+    bool has_ret_rule = false;
+    if (SPCFG(disabled_functions_ret) &&
+        zend_hash_str_find_ptr(SPCFG(disabled_functions_ret), fname, strlen(fname))) {
+      has_ret_rule = true;
+    }
+    if (!has_ret_rule && SPCFG(disabled_functions_reg_ret).disabled_functions) {
+      has_ret_rule = true;  /* regex rules require runtime matching */
+    }
+    if (has_ret_rule) {
+      op->fn_flags |= ZEND_ACC_HAS_TYPE_HINTS;
+    }
+    efree(fname);
+  }
+#endif
 }
 
 ZEND_DECLARE_MODULE_GLOBALS(snuffleupagus)
@@ -246,7 +279,7 @@ PHP_MINFO_FUNCTION(snuffleupagus) {
   php_info_print_table_start();
   php_info_print_table_row(
       2, "snuffleupagus support",
-      SPG(is_config_valid) ? "enabled" : "disabled");
+      SPG(is_config_valid) == SP_CONFIG_VALID ? "enabled" : "disabled");
   php_info_print_table_row(2, "Version", PHP_SNUFFLEUPAGUS_VERSION);
   php_info_print_table_row(2, "Valid config", valid_config);
   php_info_print_table_end();
@@ -582,12 +615,25 @@ static PHP_INI_MH(OnUpdateConfiguration) {
 
   sp_hook_register_server_variables();
 
-  if (SPCFG(global_strict).enable) {
+  bool need_op_array_handler = SPCFG(global_strict).enable;
+
+#if PHP_VERSION_ID >= 80500
+  /* Register as zend extension to get op_array_handler callbacks, which we
+   * use to prevent opcache from inlining monitored functions. */
+  if (SPCFG(disabled_functions_ret) && zend_hash_num_elements(SPCFG(disabled_functions_ret))) {
+    need_op_array_handler = true;
+  }
+  if (SPCFG(disabled_functions_reg_ret).disabled_functions) {
+    need_op_array_handler = true;
+  }
+#endif
+
+  if (need_op_array_handler) {
     if (!zend_get_extension(PHP_SNUFFLEUPAGUS_EXTNAME)) {
       zend_extension_entry.startup = NULL;
       zend_register_extension(&zend_extension_entry, NULL);
     }
-    // This is needed to implement the global strict mode
+    // This is needed to enable the op_array_handler callback
     CG(compiler_options) |= ZEND_COMPILE_HANDLE_OP_ARRAY;
   }
 
diff --git a/src/sp_config_utils.c b/src/sp_config_utils.c
index 84b1f30..415e434 100644
--- a/src/sp_config_utils.c
+++ b/src/sp_config_utils.c
@@ -11,8 +11,9 @@ sp_list_node *parse_functions_list(const char *const value) {
   sp_list_node *list = NULL;
   char *tmp = strdup(value);
   const char *function_name;
-  char *next_token = tmp;
-  while ((function_name = strtok_r(NULL, sep, &next_token))) {
+  char *next_token = NULL;
+  for (function_name = strtok_r(tmp, sep, &next_token); function_name;
+       function_name = strtok_r(NULL, sep, &next_token)) {
     list = sp_list_prepend(list, strdup(function_name));
   }
   free(tmp);
diff --git a/src/sp_cookie_encryption.c b/src/sp_cookie_encryption.c
index 0c14c70..7b3e088 100644
--- a/src/sp_cookie_encryption.c
+++ b/src/sp_cookie_encryption.c
@@ -31,6 +31,11 @@ int decrypt_cookie(zval *pDest, int num_args, va_list args,
     return ZEND_HASH_APPLY_KEEP;
   }
 
+  /* Cookies can be arrays, like session_id[]=x */
+  if (Z_TYPE_P(pDest) != IS_STRING) {
+    return ZEND_HASH_APPLY_KEEP;
+  }
+
   /* If the cookie has no value, it shouldn't be encrypted. */
   if (0 == Z_STRLEN_P(pDest)) {
     return ZEND_HASH_APPLY_KEEP;
diff --git a/src/sp_crypt.c b/src/sp_crypt.c
index 6d48554..3b65616 100644
--- a/src/sp_crypt.c
+++ b/src/sp_crypt.c
@@ -32,6 +32,7 @@ void generate_key(unsigned char *key) {
   }
 
   PHP_SHA256Final((unsigned char *)key, &ctx);
+  ZEND_SECURE_ZERO(&ctx, sizeof(ctx));
 }
 
 // This function return 0 upon success , non-zero otherwise
@@ -42,6 +43,11 @@ int decrypt_zval(zval *pDest, bool simulation, zend_hash_key *hash_key) {
 
   zend_string *debase64 = php_base64_decode((unsigned char *)(Z_STRVAL_P(pDest)), Z_STRLEN_P(pDest));
 
+  if (!debase64) {
+      sp_log_drop( "cookie_encryption", "Unable to base64-decode the cookie");
+      return ZEND_HASH_APPLY_REMOVE;
+  }
+
   if (ZSTR_LEN(debase64) < crypto_secretbox_NONCEBYTES) {
     if (true == simulation) {
       sp_log_simulation(
@@ -115,6 +121,7 @@ int decrypt_zval(zval *pDest, bool simulation, zend_hash_key *hash_key) {
   ret = ZEND_HASH_APPLY_KEEP;
 
 out:
+  ZEND_SECURE_ZERO(key, sizeof(key));
   zend_string_efree(debase64);
   efree(decrypted);
   efree(backup);
@@ -164,6 +171,8 @@ zend_string *encrypt_zval(zend_string *data) {
     z = php_base64_encode(encrypted_data, emsg_and_nonce_len);
   }
 
+  ZEND_SECURE_ZERO(key, sizeof(key));
+  ZEND_SECURE_ZERO(nonce, sizeof(nonce));
   efree(data_to_encrypt);
   efree(encrypted_data);
 
diff --git a/src/sp_ifilter.c b/src/sp_ifilter.c
index 67eb5f3..ffdeec1 100644
--- a/src/sp_ifilter.c
+++ b/src/sp_ifilter.c
@@ -33,7 +33,7 @@ static void sp_server_strip(HashTable *svars, const char *key, size_t keylen) {
   char *tmpend = tmp + ZSTR_LEN(tmp_zstr);
 
   for (char *p = tmp; p < tmpend; p++) {
-    if (sp_is_dangerous_char[(int)*p]) {
+    if (sp_is_dangerous_char[(unsigned char)*p]) {
       *p = '_';
     }
   }
@@ -49,17 +49,17 @@ static void sp_server_encode(HashTable *svars, const char *key, size_t keylen) {
   int extra = 0;
   
   for (char *p = tmp; p < tmpend; p++) {
-    extra += sp_is_dangerous_char[(int)*p] * 2;
+    extra += sp_is_dangerous_char[(unsigned char)*p] * 2;
   }
   if (!extra) { return; }
 
   zend_string *new_zstr = zend_string_alloc(ZSTR_LEN(tmp_zstr) + extra, 0);
   char *n = ZSTR_VAL(new_zstr);
   for (char *p = tmp; p < tmpend; p++, n++) {
-    if (sp_is_dangerous_char[(int)*p]) {
+    if (sp_is_dangerous_char[(unsigned char)*p]) {
       *n++ = '%';
-      *n++ = sp_hexchars[*p >> 4];
-      *n = sp_hexchars[*p & 15];
+      *n++ = sp_hexchars[(unsigned char)*p >> 4];
+      *n = sp_hexchars[(unsigned char)*p & 15];
     } else {
       *n = *p;
     }
diff --git a/src/sp_upload_validation.c b/src/sp_upload_validation.c
index 4ac4992..b5babed 100644
--- a/src/sp_upload_validation.c
+++ b/src/sp_upload_validation.c
@@ -54,8 +54,9 @@ static int sp_rfc1867_callback(unsigned int event, void *event_data, void **extr
       cmd[1] = tmp_name;
       cmd[2] = NULL;
 
+      const char *remote_addr = getenv("REMOTE_ADDR");
       spprintf(&env[0], 0, "SP_FILENAME=%s", filename);
-      spprintf(&env[1], 0, "SP_REMOTE_ADDR=%s", getenv("REMOTE_ADDR"));
+      spprintf(&env[1], 0, "SP_REMOTE_ADDR=%s", remote_addr ? remote_addr : "");
       spprintf(&env[2], 0, "SP_CURRENT_FILE=%s", zend_get_executed_filename(TSRMLS_C));
       spprintf(&env[3], 0, "SP_FILESIZE=%zu", filesize);
       env[4] = NULL;
@@ -63,8 +64,7 @@ static int sp_rfc1867_callback(unsigned int event, void *event_data, void **extr
       if ((pid = fork()) == 0) {
         if (execve(ZSTR_VAL(config_upload->script), cmd, env) == -1) {
           sp_log_warn("upload_validation", "Could not call '%s' : %s", ZSTR_VAL(config_upload->script), strerror(errno));
-          EFREE_3(env);
-          exit(1);
+          _exit(1);
         }
       } else if (pid == -1) {
         // LCOV_EXCL_START
@@ -76,7 +76,7 @@ static int sp_rfc1867_callback(unsigned int event, void *event_data, void **extr
 
       EFREE_3(env);
       int waitstatus;
-      wait(&waitstatus);
+      waitpid(pid, &waitstatus, 0);
       if (WEXITSTATUS(waitstatus) != 0) {  // Nope
         char *uri = getenv("REQUEST_URI");
         int sim = config_upload->simulation;
diff --git a/src/sp_utils.c b/src/sp_utils.c
index d49d459..10beb8b 100644
--- a/src/sp_utils.c
+++ b/src/sp_utils.c
@@ -315,11 +315,11 @@ void sp_log_disable(const char* restrict path, const char* restrict arg_name,
   if (arg_name) {
     char* char_repr = NULL;
     if (arg_value) {
-      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));
+      zend_string *arg_value_enc = php_raw_url_encode(ZSTR_VAL(arg_value), ZSTR_LEN(arg_value));
+      char_repr = zend_string_to_char(arg_value_enc);
+      size_t max_len = MIN(ZSTR_LEN(arg_value_enc), (size_t)SPCFG(log_max_len));
       char_repr[max_len] = '\0';
+      zend_string_release(arg_value_enc);
     }
     if (alias) {
       sp_log_auto(
@@ -359,11 +359,11 @@ void sp_log_disable_ret(const char* restrict path,
     sp_log_request(dump, config_node->textual_representation);
   }
   if (ret_value) {
-    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));
+    zend_string *ret_value_enc = php_raw_url_encode(ZSTR_VAL(ret_value), ZSTR_LEN(ret_value));
+    char_repr = zend_string_to_char(ret_value_enc);
+    size_t max_len = MIN(ZSTR_LEN(ret_value_enc), (size_t)SPCFG(log_max_len));
     char_repr[max_len] = '\0';
+    zend_string_release(ret_value_enc);
   }
   if (alias) {
     sp_log_auto(
@@ -393,11 +393,12 @@ bool sp_match_array_key(const zval* zv, const zend_string* to_match, const sp_re
       char* idx_str = NULL;
       spprintf(&idx_str, 0, ZEND_ULONG_FMT, idx);
       zend_string* tmp = zend_string_init(idx_str, strlen(idx_str), 0);
-      if (sp_match_value(tmp, to_match, rx)) {
-        efree(idx_str);
+      bool match = sp_match_value(tmp, to_match, rx);
+      zend_string_release(tmp);
+      efree(idx_str);
+      if (match) {
         return true;
       }
-      efree(idx_str);
     }
   }
   ZEND_HASH_FOREACH_END();
diff --git a/src/sp_wrapper.c b/src/sp_wrapper.c
index 6b6c5cd..22b1d88 100644
--- a/src/sp_wrapper.c
+++ b/src/sp_wrapper.c
@@ -180,7 +180,7 @@ PHP_FUNCTION(sp_stream_wrapper_register) {
   if (!protocol_name || wrapper_is_whitelisted(protocol_name)) {
 
     // reject manual loading of "php" wrapper
-    if (!strcasecmp(ZSTR_VAL(protocol_name), "php") && sp_php_stream_is_filtered()) {
+    if (protocol_name && !strcasecmp(ZSTR_VAL(protocol_name), "php") && sp_php_stream_is_filtered()) {
       return;
     }
 
diff --git a/src/tests/disable_function/disabled_function_echo_2.phpt b/src/tests/disable_function/disabled_function_echo_2.phpt
index c1d9817..ce3488e 100644
--- a/src/tests/disable_function/disabled_function_echo_2.phpt
+++ b/src/tests/disable_function/disabled_function_echo_2.phpt
@@ -4,11 +4,13 @@ Echo hooking
 <?php if (!extension_loaded("snuffleupagus")) print "skip"; ?>
 --INI--
 sp.configuration_file={PWD}/config/disabled_function_echo.ini
+opcache.optimization_level=0
 --FILE--
 <?php 
 echo "qwe";
-echo "1", "oops";
+echo "1";
+echo "oops";
 ?>
 --EXPECTF--
 qwe1
-Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'echo' in %a/disabled_function_echo_2.php on line 3
+Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'echo' in %a/disabled_function_echo_2.php on line 4
diff --git a/src/tests/unserialize_php8/equivalent.phpt b/src/tests/unserialize_php8/equivalent.phpt
new file mode 100644
index 0000000..57dcf72
--- /dev/null
+++ b/src/tests/unserialize_php8/equivalent.phpt
@@ -0,0 +1,23 @@
+--TEST--
+Test idempotence of serialize+unserialize
+--SKIPIF--
+<?php if (!extension_loaded("snuffleupagus")) print "skip"; ?>
+--INI--
+sp.configuration_file={PWD}/config/config_serialize.ini
+--FILE--
+<?php 
+class A {
+public $pub = "public data";
+protected $prot = "protected data";
+private $priv = "private data";
+}
+$a = new A;
+if (unserialize(serialize($a)) == $a) {
+        echo "OK";
+} else {
+        echo "FAIL";
+}
+?>
+--EXPECT--
+OK
+