Rework the priority of bl/wl in eval

7 files changed, 73 insertions, 31 deletions

diff --git a/src/sp_disabled_functions.c b/src/sp_disabled_functions.c
index fa9d625..07c8696 100644
--- a/src/sp_disabled_functions.c
+++ b/src/sp_disabled_functions.c
@@ -5,9 +5,7 @@
 
 ZEND_DECLARE_MODULE_GLOBALS(snuffleupagus)
 
-
-char* get_complete_function_path(
-    zend_execute_data const* const execute_data) {
+char* get_complete_function_path(zend_execute_data const* const execute_data) {
   if (zend_is_executing() && !EG(current_execute_data)->func) {
     return NULL;
   }
@@ -469,6 +467,10 @@ ZEND_FUNCTION(eval_blacklist_callback) {
   void (*orig_handler)(INTERNAL_FUNCTION_PARAMETERS);
   const char* current_function_name = get_active_function_name(TSRMLS_C);
 
+  if (true == check_is_in_eval_whitelist(current_function_name)) {
+    goto whitelisted;
+  }
+
   if (SNUFFLEUPAGUS_G(in_eval) > 0) {
     const char* filename = get_eval_filename(zend_get_executed_filename());
     const int line_number = zend_get_executed_lineno(TSRMLS_C);
@@ -484,6 +486,7 @@ ZEND_FUNCTION(eval_blacklist_callback) {
     }
   }
 
+whitelisted:
   orig_handler = zend_hash_str_find_ptr(
       SNUFFLEUPAGUS_G(sp_eval_blacklist_functions_hook), current_function_name,
       strlen(current_function_name));
diff --git a/src/sp_execute.c b/src/sp_execute.c
index e6df1b6..ac7cee3 100644
--- a/src/sp_execute.c
+++ b/src/sp_execute.c
@@ -44,7 +44,8 @@ static void is_builtin_matching(const char *restrict const filename,
   }
 }
 
-static void is_in_eval_and_whitelisted(const zend_execute_data *execute_data) {
+static void ZEND_HOT
+is_in_eval_and_whitelisted(const zend_execute_data *execute_data) {
   if (EXPECTED(0 == SNUFFLEUPAGUS_G(in_eval))) {
     return;
   }
@@ -61,25 +62,16 @@ static void is_in_eval_and_whitelisted(const zend_execute_data *execute_data) {
     return;
   }
 
-  char const* const current_function = ZSTR_VAL(EX(func)->common.function_name);
+  char const *const current_function = ZSTR_VAL(EX(func)->common.function_name);
 
-  if (EXPECTED(current_function)) {
-    const sp_list_node *it = SNUFFLEUPAGUS_G(config).config_eval->whitelist;
-    /* yes, we could use a HashTable instead, but since the list is pretty
-     * small, it doesn't maka a difference in practise. */
-    while (it) {
-      if (0 == strcmp(current_function, (char *)(it->data))) {
-        /* We've got a match, the function is whiteslited. */
-        return;
-      }
-      it = it->next;
+  if (EXPECTED(NULL != current_function)) {
+    if (false == check_is_in_eval_whitelist(current_function)) {
+      sp_log_msg(
+          "Eval_whitelist", SP_LOG_DROP,
+          "The function '%s' isn't in the eval whitelist, dropping its call.",
+          current_function);
+      sp_terminate();
     }
-
-    sp_log_msg(
-        "Eval_whitelist", SP_LOG_DROP,
-        "The function '%s' isn't in the eval whitelist, dropping its call.",
-        current_function);
-    sp_terminate();
   }
 }
 
@@ -104,6 +96,8 @@ char *get_eval_filename(const char *filename) {
 }
 
 static void sp_execute_ex(zend_execute_data *execute_data) {
+  is_in_eval_and_whitelisted(execute_data);
+
   if (true == should_disable(execute_data, NULL, NULL, NULL)) {
     sp_terminate();
   }
@@ -117,8 +111,6 @@ static void sp_execute_ex(zend_execute_data *execute_data) {
     efree(filename);
   }
 
-  is_in_eval_and_whitelisted(execute_data);
-
   if (NULL != EX(func)->op_array.filename) {
     if (true == SNUFFLEUPAGUS_G(config).config_readonly_exec->enable) {
       terminate_if_writable(ZSTR_VAL(EX(func)->op_array.filename));
@@ -176,7 +168,7 @@ static int sp_stream_open(const char *filename, zend_file_handle *handle) {
           is_builtin_matching(filename, "include_once", "inclusion path",
                               config);
           break;
-        EMPTY_SWITCH_DEFAULT_CASE();
+          EMPTY_SWITCH_DEFAULT_CASE();
       }
   }
 
diff --git a/src/sp_utils.c b/src/sp_utils.c
index 8a1ed87..19b7179 100644
--- a/src/sp_utils.c
+++ b/src/sp_utils.c
@@ -7,6 +7,8 @@
 #include <string.h>
 #include <unistd.h>
 
+ZEND_DECLARE_MODULE_GLOBALS(snuffleupagus)
+
 static inline void _sp_log_err(const char* fmt, ...) {
   char* msg;
   va_list args;
@@ -394,3 +396,22 @@ int hook_regexp(const pcre* regexp, HashTable* hook_table,
   ZEND_HASH_FOREACH_END();
   return SUCCESS;
 }
+
+bool check_is_in_eval_whitelist(const char* const function_name) {
+  const sp_list_node* it = SNUFFLEUPAGUS_G(config).config_eval->whitelist;
+
+  if (!it->head) {
+    return false;
+  }
+
+  /* yes, we could use a HashTable instead, but since the list is pretty
+   * small, it doesn't maka a difference in practise. */
+  while (it && it->data) {
+    if (0 == strcmp(function_name, (char*)(it->data))) {
+      /* We've got a match, the function is whiteslited. */
+      return true;
+    }
+    it = it->next;
+  }
+  return false;
+}
diff --git a/src/sp_utils.h b/src/sp_utils.h
index e055e70..a7b672f 100644
--- a/src/sp_utils.h
+++ b/src/sp_utils.h
@@ -69,5 +69,6 @@ int hook_function(const char *, HashTable *,
                   void (*)(INTERNAL_FUNCTION_PARAMETERS), bool);
 int hook_regexp(const pcre *, HashTable *,
                 void (*)(INTERNAL_FUNCTION_PARAMETERS), bool);
+bool check_is_in_eval_whitelist(const char * const function_name);
 
 #endif /* SP_UTILS_H */
diff --git a/src/tests/config/eval_whitelist_blacklist.ini b/src/tests/config/eval_whitelist_blacklist.ini
new file mode 100644
index 0000000..a916004
--- /dev/null
+++ b/src/tests/config/eval_whitelist_blacklist.ini
@@ -0,0 +1,2 @@
+sp.eval_blacklist.list("my_fun,cos");
+sp.eval_whitelist.list("my_fun");
diff --git a/src/tests/eval_backlist_whitelist.phpt b/src/tests/eval_backlist_whitelist.phpt
index 1611288..9b0bb55 100644
--- a/src/tests/eval_backlist_whitelist.phpt
+++ b/src/tests/eval_backlist_whitelist.phpt
@@ -3,25 +3,21 @@ Eval whitelist
 --SKIPIF--
 <?php if (!extension_loaded("snuffleupagus")) die "skip"; ?>
 --INI--
-sp.configuration_file={PWD}/config/eval_whitelist.ini
+sp.configuration_file={PWD}/config/eval_whitelist_blacklist.ini
 --FILE--
 <?php 
 function my_fun($p) {
         return "my_fun: $p";
 }
 
-function my_other_fun($p) {
-        return "my_other_fun: $p";
-}
-
 $a = my_fun("1337 1337 1337");
 echo "Outside of eval: $a\n";
 eval('$a = my_fun("1234");');
 echo "After allowed eval: $a\n";
-eval('$a = my_other_fun("1234");');
+eval('$a = cos(1234);');
 echo "After eval: $a\n";
 ?>
 --EXPECTF--
 Outside of eval: my_fun: 1337 1337 1337
 After allowed eval: my_fun: 1234
-[snuffleupagus][0.0.0.0][Eval_whitelist][drop] The function 'my_other_fun' isn't in the eval whitelist, dropping its call.
+[snuffleupagus][0.0.0.0][Eval_whitelist][drop] The function 'cos' isn't in the eval whitelist, dropping its call.
diff --git a/src/tests/eval_whitelist.phpt b/src/tests/eval_whitelist.phpt
new file mode 100644
index 0000000..1611288
--- /dev/null
+++ b/src/tests/eval_whitelist.phpt
@@ -0,0 +1,27 @@
+--TEST--
+Eval whitelist
+--SKIPIF--
+<?php if (!extension_loaded("snuffleupagus")) die "skip"; ?>
+--INI--
+sp.configuration_file={PWD}/config/eval_whitelist.ini
+--FILE--
+<?php 
+function my_fun($p) {
+        return "my_fun: $p";
+}
+
+function my_other_fun($p) {
+        return "my_other_fun: $p";
+}
+
+$a = my_fun("1337 1337 1337");
+echo "Outside of eval: $a\n";
+eval('$a = my_fun("1234");');
+echo "After allowed eval: $a\n";
+eval('$a = my_other_fun("1234");');
+echo "After eval: $a\n";
+?>
+--EXPECTF--
+Outside of eval: my_fun: 1337 1337 1337
+After allowed eval: my_fun: 1234
+[snuffleupagus][0.0.0.0][Eval_whitelist][drop] The function 'my_other_fun' isn't in the eval whitelist, dropping its call.