8 files changed, 80 insertions, 15 deletions

diff --git a/config/default.rules b/config/default.rules
index 3e82ae3..818e73d 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();
@@ -69,6 +73,14 @@ sp.cookie.name("PHPSESSID").samesite("lax");
     sp.disable_function.function("putenv").param("assignment").value_r("GCONV_").drop()
 @end_condition;
 
+# https://github.com/php/php-src/issues/22035
+# CURLOPT_SSLENGINE = 10089 
+@condition PHP_VERSION_ID < 80000;
+    sp.disable_function.function("curl_setopt").param("option").value("10089").drop()
+@condition PHP_VERSION_ID >= 80000;
+    sp.disable_function.function("curl_setopt").param("option").value("10089").drop()
+@end_condition;
+
 # Since people are stupid enough to use `extract` on things like $_GET or $_POST, we might as well mitigate this vector
 @condition PHP_VERSION_ID < 80000;
 sp.disable_function.function("extract").pos("0").value_r("^_").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_upload_validation.c b/src/sp_upload_validation.c
index e24149e..b5babed 100644
--- a/src/sp_upload_validation.c
+++ b/src/sp_upload_validation.c
@@ -64,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
@@ -77,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 e6efcc6..10beb8b 100644
--- a/src/sp_utils.c
+++ b/src/sp_utils.c
@@ -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/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