rewrote parameter matching logic. breaks compatibility with previous versions.

16 files changed, 220 insertions, 86 deletions

diff --git a/src/sp_disabled_functions.c b/src/sp_disabled_functions.c
index c47b5cb..84d8acf 100644
--- a/src/sp_disabled_functions.c
+++ b/src/sp_disabled_functions.c
@@ -33,6 +33,8 @@ char* get_complete_function_path(zend_execute_data const* const execute_data) {
   } else {
     complete_path_function = estrdup(function_name);
   }
+  sp_log_debug("%s", complete_path_function);
+
   return complete_path_function;
 }
 
@@ -98,107 +100,105 @@ static bool is_local_var_matching(
   return false;
 }
 
+static inline const char* get_fn_arg_name(zend_function *fn, uint32_t i) {
+  if (fn->type == ZEND_USER_FUNCTION || (fn->common.fn_flags & ZEND_ACC_USER_ARG_INFO)) {
+    return ZSTR_VAL(fn->op_array.arg_info[i].name);
+  } else {
+    return fn->internal_function.arg_info[i].name;
+  }
+}
+
 static bool is_param_matching(zend_execute_data* execute_data,
                               sp_disabled_function const* const config_node,
                               const zend_string* builtin_param,
                               const char* builtin_param_name,
                               const char** arg_name,
                               const zend_string** arg_value_str) {
-  int nb_param = ZEND_CALL_NUM_ARGS(execute_data);
-  int i = 0;
-  zval* arg_value;
-
-  if (config_node->pos != -1) {
-    if (config_node->pos > nb_param - 1) {
-      char* complete_function_path = get_complete_function_path(execute_data);
-      sp_log_warn("config",
-                  "It seems that you wrote a rule filtering on the "
-                  "%d%s argument of the function '%s', but it takes only %d "
-                  "arguments. "
-                  "Matching on _all_ arguments instead.",
-                  config_node->pos, GET_SUFFIX(config_node->pos),
-                  complete_function_path, nb_param);
-      efree(complete_function_path);
-    } else {
-      i = config_node->pos;
-      nb_param = (config_node->pos) + 1;
-    }
-  }
-
+  // builtin functions
   if (builtin_param) {
     /* We're matching on a language construct (here named "builtin"),
-     * and they can only take a single argument, but PHP considers them
-     * differently than functions arguments. */
+    * and they can only take a single argument, but PHP considers them
+    * differently than functions arguments. */
     *arg_name = builtin_param_name;
     *arg_value_str = builtin_param;
     return sp_match_value(builtin_param, config_node->value,
                           config_node->r_value);
-  } else if (config_node->r_param || config_node->pos != -1) {
-    // We're matching on a function (and not a language construct)
-    for (; i < nb_param; i++) {
-      if (ZEND_USER_CODE(execute_data->func->type)) {  // yay consistency
-        *arg_name = ZSTR_VAL(execute_data->func->common.arg_info[i].name);
-      } else {
-        *arg_name = execute_data->func->internal_function.arg_info[i].name;
-      }
-      const bool pcre_matching =
-          config_node->r_param &&
-          (true == sp_is_regexp_matching(config_node->r_param, *arg_name));
+  }
 
-      /* This is the parameter name we're looking for. */
-      if (true == pcre_matching || config_node->pos != -1) {
-        arg_value = ZEND_CALL_ARG(execute_data, i + 1);
+  // safeguards
+  if (!execute_data || !execute_data->func) {
+    sp_log_debug("no execute data -> silently ignore parameter matching");
+    return false;
+  }
 
-        if (config_node->param_type) {  // Are we matching on the `type`?
-          if (config_node->param_type == Z_TYPE_P(arg_value)) {
-            return true;
-          }
-        } else if (Z_TYPE_P(arg_value) == IS_ARRAY) {
-          *arg_value_str = sp_zval_to_zend_string(arg_value);
-          if (config_node->key || config_node->r_key) {
-            if (sp_match_array_key(arg_value, config_node->key,
-                                   config_node->r_key)) {
-              return true;
-            }
-          } else if (sp_match_array_value(arg_value, config_node->value,
-                                          config_node->r_value)) {
-            return true;
-          }
-        } else {
-          *arg_value_str = sp_zval_to_zend_string(arg_value);
-          if (sp_match_value(*arg_value_str, config_node->value,
-                             config_node->r_value)) {
-            return true;
-          }
-        }
-      }
+  *arg_name = NULL;
+  int call_num_args = EX_NUM_ARGS();
+  zend_function *fn = execute_data->func;
+  int fn_num_args = fn->common.num_args;
+
+  if (!call_num_args) {
+    sp_log_debug("no call arguments -> return");
+    return false; // no arguments to check
+  }
+
+  if (config_node->pos > call_num_args - 1 || config_node->pos > fn_num_args) {
+    // trying to match argument beyond last given argument OR beyond last declared argument.
+    // this is perfectly normal for functions with
+    //   (a) optional arguments
+    //   (b) excess arguments
+    //   (c) variadic arguments which are not supported
+    return false;
+  }
+
+  zval* arg_value = NULL;
+
+  if (config_node->pos > -1) {
+    if (config_node->pos < fn_num_args) {
+      *arg_name = get_fn_arg_name(fn, config_node->pos);
     }
+    arg_value = ZEND_CALL_ARG(execute_data, config_node->pos + 1);
   } else if (config_node->param) {
     *arg_name = config_node->param->value;
     arg_value = sp_get_var_value(execute_data, config_node->param, true);
+  } else if (config_node->r_param) {
+    for (int i = 0; i < call_num_args; i++) {
+      *arg_name = get_fn_arg_name(fn, i);
+      if (true == sp_is_regexp_matching(config_node->r_param, *arg_name)) {
+        arg_value = ZEND_CALL_ARG(execute_data, i + 1);
+      }
+    }
+  }
+  
+  if (!arg_value) {
+    sp_log_debug("no argument match -> return");
+    return false;
+  }
 
-    if (arg_value) {
-      *arg_value_str = sp_zval_to_zend_string(arg_value);
-      if (config_node->param_type) {  // Are we matching on the `type`?
-        if (config_node->param_type == Z_TYPE_P(arg_value)) {
-          return true;
-        }
-      } else if (Z_TYPE_P(arg_value) == IS_ARRAY) {
-        if (config_node->key || config_node->r_key) {
-          if (sp_match_array_key(arg_value, config_node->key,
-                                 config_node->r_key)) {
-            return true;
-          }
-        } else if (sp_match_array_value(arg_value, config_node->value,
-                                        config_node->r_value)) {
-          return true;
-        }
-      } else if (sp_match_value(*arg_value_str, config_node->value,
-                                config_node->r_value)) {
+  if (config_node->param_type) {
+    if (config_node->param_type == Z_TYPE_P(arg_value)) {
+      if (!(config_node->key || config_node->r_key || config_node->value || config_node->r_value)) { // Are we matching on the `type` only?
+        sp_log_debug("arg type match only.");
         return true;
       }
+    } else {
+      sp_log_debug("arg type mismatch -> return");
+      return false;
     }
   }
+
+  *arg_value_str = sp_zval_to_zend_string(arg_value);
+  if (Z_TYPE_P(arg_value) == IS_ARRAY) {
+    if (config_node->key || config_node->r_key) {
+      if (sp_match_array_key(arg_value, config_node->key, config_node->r_key)) {
+        return true;
+      }
+    } else if (sp_match_array_value(arg_value, config_node->value, config_node->r_value)) {
+        return true;
+    }
+  } else if (sp_match_value(*arg_value_str, config_node->value, config_node->r_value)) {
+    return true;
+  }
+
   return false;
 }
 
@@ -287,6 +287,7 @@ static void should_disable(zend_execute_data* execute_data,
                            const sp_list_node* config,
                            const zend_string* current_filename) {
   char current_file_hash[SHA256_SIZE * 2 + 1] = {0};
+  // sp_log_debug("%s %s %s",  complete_function_path, builtin_param, builtin_param_name);
 
   while (config) {
     sp_disabled_function const* const config_node =
diff --git a/src/tests/disable_function/config/disabled_function_excess_args.ini b/src/tests/disable_function/config/disabled_function_excess_args.ini
new file mode 100644
index 0000000..289dc33
--- /dev/null
+++ b/src/tests/disable_function/config/disabled_function_excess_args.ini
@@ -0,0 +1 @@
+sp.disable_function.function("foo_excess_args").pos("3").value("blubb").drop()
diff --git a/src/tests/disable_function/config/disabled_function_named_args.ini b/src/tests/disable_function/config/disabled_function_named_args.ini
new file mode 100644
index 0000000..094bc0d
--- /dev/null
+++ b/src/tests/disable_function/config/disabled_function_named_args.ini
@@ -0,0 +1,12 @@
+sp.disable_function.function("foo_named_args_pos").pos("0").value("bob").drop()
+sp.disable_function.function("foo_named_args_param").param("name").value("bob").drop()
+
+sp.disable_function.function("foo_named_args_ooo_pos").pos("0").value("bob").drop()
+sp.disable_function.function("foo_named_args_ooo_param").param("name").value("bob").drop()
+
+sp.disable_function.function("foo_named_args_ooo_opt_pos").pos("2").value("green").drop()
+sp.disable_function.function("foo_named_args_ooo_opt_param").param("color").value("green").drop()
+
+sp.disable_function.function("foo_named_args_skip_pos").pos("2").value("green").drop()
+sp.disable_function.function("foo_named_args_skip_param").param("color").value("green").drop()
+
diff --git a/src/tests/disable_function/config/disabled_functions_pos.ini b/src/tests/disable_function/config/disabled_functions_pos.ini
index f4c1e05..8b12fc6 100644
--- a/src/tests/disable_function/config/disabled_functions_pos.ini
+++ b/src/tests/disable_function/config/disabled_functions_pos.ini
@@ -1,4 +1,4 @@
 sp.disable_function.function("system").pos("1337").value("id").drop();
 sp.disable_function.function("system").pos("0").value("id").drop();
-sp.disable_function.function("system").pos("1").param_type("ARRAY").alias("1").drop();
+sp.disable_function.function("system").pos("0").param_type("ARRAY").alias("1").drop();
 sp.disable_function.function("strtoupper").pos("0").value("id").alias("strlen array").drop();
diff --git a/src/tests/disable_function/disabled_function_excess_args.phpt b/src/tests/disable_function/disabled_function_excess_args.phpt
new file mode 100644
index 0000000..31b3f33
--- /dev/null
+++ b/src/tests/disable_function/disabled_function_excess_args.phpt
@@ -0,0 +1,14 @@
+--TEST--
+Disable functions with excess arguments
+--SKIPIF--
+<?php if (!extension_loaded("snuffleupagus")) print "skip"; ?>
+--INI--
+sp.configuration_file={PWD}/config/disabled_function_excess_args.ini
+--FILE--
+<?php
+function foo_excess_args($name, $greeting='HI!', $color='red') {
+    echo "boo\n";
+}
+foo_excess_args("bob", "hi", "green", "blubb");
+--EXPECTF--
+Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'foo_excess_args' in %s.php on line %d
\ No newline at end of file
diff --git a/src/tests/disable_function/disabled_function_named_args_ooo_opt_param.phpt b/src/tests/disable_function/disabled_function_named_args_ooo_opt_param.phpt
new file mode 100644
index 0000000..6ca49d4
--- /dev/null
+++ b/src/tests/disable_function/disabled_function_named_args_ooo_opt_param.phpt
@@ -0,0 +1,14 @@
+--TEST--
+Disable functions with out-of-order named optional arguments by matching argument name
+--SKIPIF--
+<?php if (!extension_loaded("snuffleupagus") || PHP_VERSION_ID < 80000) print "skip"; ?>
+--INI--
+sp.configuration_file={PWD}/config/disabled_function_named_args.ini
+--FILE--
+<?php
+function foo_named_args_ooo_opt_param($name, $greeting='HI!', $color='red') {
+    echo "boo\n";
+}
+foo_named_args_ooo_opt_param("bob", color: "green", greeting:  "xxx");
+--EXPECTF--
+Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'foo_named_args_ooo_opt_param', because its argument '$color'%s matched a rule in %s.php on line %d
\ No newline at end of file
diff --git a/src/tests/disable_function/disabled_function_named_args_ooo_opt_pos.phpt b/src/tests/disable_function/disabled_function_named_args_ooo_opt_pos.phpt
new file mode 100644
index 0000000..86abbee
--- /dev/null
+++ b/src/tests/disable_function/disabled_function_named_args_ooo_opt_pos.phpt
@@ -0,0 +1,14 @@
+--TEST--
+Disable functions with out-of-order named optional arguments by matching argument position
+--SKIPIF--
+<?php if (!extension_loaded("snuffleupagus") || PHP_VERSION_ID < 80000) print "skip"; ?>
+--INI--
+sp.configuration_file={PWD}/config/disabled_function_named_args.ini
+--FILE--
+<?php
+function foo_named_args_ooo_opt_pos($name, $greeting='HI!', $color='red') {
+    echo "boo\n";
+}
+foo_named_args_ooo_opt_pos("bob", color: "green", greeting:  "xxx");
+--EXPECTF--
+Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'foo_named_args_ooo_opt_pos', because its argument 'color'%s matched a rule in %s.php on line %d
\ No newline at end of file
diff --git a/src/tests/disable_function/disabled_function_named_args_ooo_param.phpt b/src/tests/disable_function/disabled_function_named_args_ooo_param.phpt
new file mode 100644
index 0000000..a77fc09
--- /dev/null
+++ b/src/tests/disable_function/disabled_function_named_args_ooo_param.phpt
@@ -0,0 +1,14 @@
+--TEST--
+Disable functions with out-of-order named arguments by matching argument name
+--SKIPIF--
+<?php if (!extension_loaded("snuffleupagus") || PHP_VERSION_ID < 80000) print "skip"; ?>
+--INI--
+sp.configuration_file={PWD}/config/disabled_function_named_args.ini
+--FILE--
+<?php
+function foo_named_args_ooo_param($name, $greeting='HI!', $color='red') {
+    echo "boo\n";
+}
+foo_named_args_ooo_param(greeting: "hello!", name: "bob");
+--EXPECTF--
+Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'foo_named_args_ooo_param', because its argument '$name'%s matched a rule in %s.php on line %d
\ No newline at end of file
diff --git a/src/tests/disable_function/disabled_function_named_args_ooo_pos.phpt b/src/tests/disable_function/disabled_function_named_args_ooo_pos.phpt
new file mode 100644
index 0000000..ab7cc00
--- /dev/null
+++ b/src/tests/disable_function/disabled_function_named_args_ooo_pos.phpt
@@ -0,0 +1,14 @@
+--TEST--
+Disable functions with out-of-order named arguments by matching argument position
+--SKIPIF--
+<?php if (!extension_loaded("snuffleupagus") || PHP_VERSION_ID < 80000) print "skip"; ?>
+--INI--
+sp.configuration_file={PWD}/config/disabled_function_named_args.ini
+--FILE--
+<?php
+function foo_named_args_ooo_pos($name, $greeting='HI!', $color='red') {
+    echo "boo\n";
+}
+foo_named_args_ooo_pos(greeting: "hello!", name: "bob");
+--EXPECTF--
+Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'foo_named_args_ooo_pos', because its argument 'name'%s matched a rule in %s.php on line %d
\ No newline at end of file
diff --git a/src/tests/disable_function/disabled_function_named_args_param.phpt b/src/tests/disable_function/disabled_function_named_args_param.phpt
new file mode 100644
index 0000000..f07e736
--- /dev/null
+++ b/src/tests/disable_function/disabled_function_named_args_param.phpt
@@ -0,0 +1,14 @@
+--TEST--
+Disable functions with named arguments by matching argument name
+--SKIPIF--
+<?php if (!extension_loaded("snuffleupagus") || PHP_VERSION_ID < 80000) print "skip"; ?>
+--INI--
+sp.configuration_file={PWD}/config/disabled_function_named_args.ini
+--FILE--
+<?php
+function foo_named_args_param($name, $greeting='HI!', $color='red') {
+    echo "boo\n";
+}
+foo_named_args_param(name: "bob", greeting: "hello!");
+--EXPECTF--
+Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'foo_named_args_param', because its argument '$name'%s matched a rule in %s.php on line %d
\ No newline at end of file
diff --git a/src/tests/disable_function/disabled_function_named_args_pos.phpt b/src/tests/disable_function/disabled_function_named_args_pos.phpt
new file mode 100644
index 0000000..c0189fe
--- /dev/null
+++ b/src/tests/disable_function/disabled_function_named_args_pos.phpt
@@ -0,0 +1,14 @@
+--TEST--
+Disable functions with named arguments by matching argument position
+--SKIPIF--
+<?php if (!extension_loaded("snuffleupagus") || PHP_VERSION_ID < 80000) print "skip"; ?>
+--INI--
+sp.configuration_file={PWD}/config/disabled_function_named_args.ini
+--FILE--
+<?php
+function foo_named_args_pos($name, $greeting='HI!', $color='red') {
+    echo "boo\n";
+}
+foo_named_args_pos(name: "bob", greeting: "hello!");
+--EXPECTF--
+Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'foo_named_args_pos', because its argument 'name'%s matched a rule in %s.php on line %d
\ No newline at end of file
diff --git a/src/tests/disable_function/disabled_function_named_args_skip_param.phpt b/src/tests/disable_function/disabled_function_named_args_skip_param.phpt
new file mode 100644
index 0000000..86b1a5e
--- /dev/null
+++ b/src/tests/disable_function/disabled_function_named_args_skip_param.phpt
@@ -0,0 +1,14 @@
+--TEST--
+Disable functions with named arguments (skipping opt. args) by matching argument name
+--SKIPIF--
+<?php if (!extension_loaded("snuffleupagus") || PHP_VERSION_ID < 80000) print "skip"; ?>
+--INI--
+sp.configuration_file={PWD}/config/disabled_function_named_args.ini
+--FILE--
+<?php
+function foo_named_args_skip_param($name, $greeting='HI!', $color='red') {
+    echo "boo\n";
+}
+foo_named_args_skip_param("bob", color: "green");
+--EXPECTF--
+Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'foo_named_args_skip_param', because its argument '$color'%s matched a rule in %s.php on line %d
\ No newline at end of file
diff --git a/src/tests/disable_function/disabled_function_named_args_skip_pos.phpt b/src/tests/disable_function/disabled_function_named_args_skip_pos.phpt
new file mode 100644
index 0000000..ce85241
--- /dev/null
+++ b/src/tests/disable_function/disabled_function_named_args_skip_pos.phpt
@@ -0,0 +1,14 @@
+--TEST--
+Disable functions with named arguments (skipping opt. args) by matching argument position
+--SKIPIF--
+<?php if (!extension_loaded("snuffleupagus") || PHP_VERSION_ID < 80000) print "skip"; ?>
+--INI--
+sp.configuration_file={PWD}/config/disabled_function_named_args.ini
+--FILE--
+<?php
+function foo_named_args_skip_pos($name, $greeting='HI!', $color='red') {
+    echo "boo\n";
+}
+foo_named_args_skip_pos("bob", color: "green");
+--EXPECTF--
+Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'foo_named_args_skip_pos', because its argument 'color'%s matched a rule in %s.php on line %d
\ No newline at end of file
diff --git a/src/tests/disable_function/disabled_functions_name_type.phpt b/src/tests/disable_function/disabled_functions_name_type.phpt
index 8d70eaa..62a6328 100644
--- a/src/tests/disable_function/disabled_functions_name_type.phpt
+++ b/src/tests/disable_function/disabled_functions_name_type.phpt
@@ -13,4 +13,4 @@ echo strcmp([1,23], "pouet") . "\n";
 --EXPECTF--
 0
 
-Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'strcmp', because its argument '$str1' content (ARRAY) matched a rule in %a/disabled_functions_name_type.php on line 3
+Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'strcmp', because its argument '$str1'%smatched a rule in %s/disabled_functions_name_type.php on line 3
diff --git a/src/tests/disable_function/disabled_functions_param_pos.phpt b/src/tests/disable_function/disabled_functions_param_pos.phpt
index bacca62..4204548 100644
--- a/src/tests/disable_function/disabled_functions_param_pos.phpt
+++ b/src/tests/disable_function/disabled_functions_param_pos.phpt
@@ -9,6 +9,4 @@ sp.configuration_file={PWD}/config/disabled_functions_pos.ini
 system("id");
 ?>
 --EXPECTF--
-Warning: [snuffleupagus][0.0.0.0][config][log] It seems that you wrote a rule filtering on the 1337th argument of the function 'system', but it takes only 1 arguments. Matching on _all_ arguments instead. in %a/disabled_functions_param_pos.php on line 2
-
 Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'system', because its argument 'command' content (id) matched a rule in %a/disabled_functions_param_pos.php on line %d
diff --git a/src/tests/disable_function/disabled_functions_pos_type.phpt b/src/tests/disable_function/disabled_functions_pos_type.phpt
index ba134ad..29944c0 100644
--- a/src/tests/disable_function/disabled_functions_pos_type.phpt
+++ b/src/tests/disable_function/disabled_functions_pos_type.phpt
@@ -9,8 +9,4 @@ sp.configuration_file={PWD}/config/disabled_functions_pos.ini
 system([123, 456]);
 ?>
 --EXPECTF--
-Warning: [snuffleupagus][0.0.0.0][config][log] It seems that you wrote a rule filtering on the 1337th argument of the function 'system', but it takes only 1 arguments. Matching on _all_ arguments instead. in %a/disabled_functions_pos_type.php on line %d
-
-Warning: [snuffleupagus][0.0.0.0][config][log] It seems that you wrote a rule filtering on the 1st argument of the function 'system', but it takes only 1 arguments. Matching on _all_ arguments instead. in %a/disabled_functions_pos_type.php on line %d
-
 Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'system', because its argument 'command' content (?) matched the rule '1' in %a/disabled_functions_pos_type.php on line %d