From 92a4b93c4d420fefe590bd88521ec76d8bebd3fe Mon Sep 17 00:00:00 2001
From: jvoisin
Date: Tue, 24 Oct 2017 00:16:30 +0200
Subject: Remove the `enable` member from the disable function structure

Also add some more tests
---
 src/sp_config.h                                    |  1 -
 src/sp_config_keywords.c                           | 10 ++++------
 src/sp_disabled_functions.c                        | 23 +++++++++++-----------
 .../config/config_disabled_functions_param_r.ini   |  1 +
 src/tests/config/disabled_functions_pos.ini        |  1 +
 src/tests/config/disabled_functions_ret.ini        |  1 +
 src/tests/disabled_functions_param_pos.phpt        |  3 ++-
 7 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/src/sp_config.h b/src/sp_config.h
index c6de8ba..eda7517 100644
--- a/src/sp_config.h
+++ b/src/sp_config.h
@@ -72,7 +72,6 @@ typedef struct {
 
   char *hash;
   int simulation;
-  bool enable;
 
   char *param;
   pcre *r_param;
diff --git a/src/sp_config_keywords.c b/src/sp_config_keywords.c
index 168ee1c..604c2a1 100644
--- a/src/sp_config_keywords.c
+++ b/src/sp_config_keywords.c
@@ -182,12 +182,6 @@ int parse_disabled_functions(char *line) {
     return ret;
   }
 
-  if (true == disable){
-    df->enable = false;
-  } else {
-    df->enable = true;
-  }
-
   if (df->value && df->value_r) {
     sp_log_err("config",
                "Invalid configuration line: 'sp.disabled_functions%s':"
@@ -296,6 +290,10 @@ int parse_disabled_functions(char *line) {
       break;
   }
 
+  if (true == disable) {
+    return ret;
+  }
+
   if (df->ret || df->r_ret || df->ret_type) {
     sp_list_insert(
         SNUFFLEUPAGUS_G(config).config_disabled_functions_ret->disabled_functions,
diff --git a/src/sp_disabled_functions.c b/src/sp_disabled_functions.c
index f089c25..f0b785c 100644
--- a/src/sp_disabled_functions.c
+++ b/src/sp_disabled_functions.c
@@ -124,10 +124,6 @@ bool should_disable(zend_execute_data* execute_data) {
     const char* arg_name = NULL;
     const char* arg_value_str = NULL;
 
-    if (false == config_node->enable) {
-      goto next;
-    }
-
     /* The order matters, since when we have `config_node->functions_list`,
     we also do have `config_node->function` */
     if (config_node->functions_list) {
@@ -189,9 +185,18 @@ bool should_disable(zend_execute_data* execute_data) {
       bool arg_matched = false;
       int i = 0;
 
-      if ((config_node->pos != -1) && (config_node->pos <= nb_param)) {
-        i = config_node->pos;
-        nb_param = (config_node->pos) + 1;
+      if (config_node->pos != -1) {
+        if (config_node->pos <= nb_param) {
+          sp_log_err("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,
+            (config_node->pos == 1)?"st":(config_node->pos)?"nd":"th",
+            complete_path_function, nb_param);
+        } else {
+          i = config_node->pos;
+          nb_param = (config_node->pos) + 1;
+        }
       }
 
       for (; i < nb_param; i++) {
@@ -301,10 +306,6 @@ static bool should_drop_on_ret(zval* return_value,
     sp_disabled_function const* const config_node =
         (sp_disabled_function*)(config->data);
 
-    if (false == config_node->enable) {
-      goto next;
-    }
-
     if (config_node->function) {
       if (0 != strcmp(config_node->function, complete_path_function)) {
         goto next;
diff --git a/src/tests/config/config_disabled_functions_param_r.ini b/src/tests/config/config_disabled_functions_param_r.ini
index 8e9ac63..09a59fe 100644
--- a/src/tests/config/config_disabled_functions_param_r.ini
+++ b/src/tests/config/config_disabled_functions_param_r.ini
@@ -1 +1,2 @@
+sp.disable_function.function("system").param_r("^not_command$").value("id").drop();
 sp.disable_function.function("system").param_r("^command$").value("id").drop();
diff --git a/src/tests/config/disabled_functions_pos.ini b/src/tests/config/disabled_functions_pos.ini
index f96cf3d..e7d12a9 100644
--- a/src/tests/config/disabled_functions_pos.ini
+++ b/src/tests/config/disabled_functions_pos.ini
@@ -1 +1,2 @@
+sp.disable_function.function("system").pos("1337").value("id").drop();
 sp.disable_function.function("system").pos("0").value("id").drop();
diff --git a/src/tests/config/disabled_functions_ret.ini b/src/tests/config/disabled_functions_ret.ini
index 4afcd34..288177a 100644
--- a/src/tests/config/disabled_functions_ret.ini
+++ b/src/tests/config/disabled_functions_ret.ini
@@ -1,4 +1,5 @@
 sp.disable_function.function("testFunction").ret("0").drop().disable();
+sp.disable_function.function("strpos").ret("0").drop().filename_r(".*\\.not_matching");
 sp.disable_function.function("strpos").ret("0").drop().filename_r(".*\\.php");
 sp.disable_function.function_r("str[ia]pos").ret_r("^[^a-z]+$").drop();
 sp.disable_function.function_r("stripos").ret_r("^[^a-z]+").drop();
diff --git a/src/tests/disabled_functions_param_pos.phpt b/src/tests/disabled_functions_param_pos.phpt
index de578b2..a1f8895 100644
--- a/src/tests/disabled_functions_param_pos.phpt
+++ b/src/tests/disabled_functions_param_pos.phpt
@@ -9,4 +9,5 @@ sp.configuration_file={PWD}/config/disabled_functions_pos.ini
 system("id");
 ?>
 --EXPECTF--
-[snuffleupagus][0.0.0.0][disabled_function][drop] The call to the function 'system' in %a/disabled_functions_param_pos.php:%d has been disabled, because its argument 'command' content (id) matched a rule.
+[snuffleupagus][0.0.0.0][config][error] It seems that you wrote a rule filtering on the 0th argument of the function 'system', but it takes only 2 arguments. Matching on _all_ arguments instead.
+[snuffleupagus][0.0.0.0][disabled_function][drop] The call to the function 'system' in %a/disabled_functions_param_pos.php:2 has been disabled, because its argument 'command' content (id) matched a rule.
-- 
cgit v1.3