Eval whitelist

Implement whitelist in eval

18 files changed, 208 insertions, 43 deletions

diff --git a/doc/source/config.rst b/doc/source/config.rst
index d16474f..602b78d 100644
--- a/doc/source/config.rst
+++ b/doc/source/config.rst
@@ -251,13 +251,14 @@ Eval white and blacklist
  * `default: disabled`
  * :ref:`more <eval-feature>`
 
-``eval_filter`` allows to specify white and blacklist of functions allowed and
-forbidden from being called inside ``eval``. The functions names are comma-separated.
+``eval_whitelist`` and ``eval_blacklist`` allow to respectively specify
+functions allowed and forbidden from being called inside ``eval``. The
+functions names are comma-separated.
 
 ::
 
-  sp.eval_filter.blacklist("system,exec,shell_exec");
-  sp.eval_filter.whitelist("strlen,strcmp").simulation();
+  sp.eval_blacklist.list("system,exec,shell_exec");
+  sp.eval_whitelist.list("strlen,strcmp").simulation();
 
 
 Virtual-patching
diff --git a/src/php_snuffleupagus.h b/src/php_snuffleupagus.h
index ca39bb8..93ef472 100644
--- a/src/php_snuffleupagus.h
+++ b/src/php_snuffleupagus.h
@@ -63,7 +63,7 @@ sp_config config;
 bool is_config_valid;
 HashTable *disabled_functions_hook;
 HashTable *sp_internal_functions_hook;
-HashTable *sp_eval_filter_functions_hook;
+HashTable *sp_eval_blacklist_functions_hook;
 ZEND_END_MODULE_GLOBALS(snuffleupagus)
 
 #define SNUFFLEUPAGUS_G(v) ZEND_MODULE_GLOBALS_ACCESSOR(snuffleupagus, v)
@@ -85,7 +85,7 @@ ZEND_TSRMLS_CACHE_EXTERN()
 #endif
 
 PHP_FUNCTION(check_disabled_function);
-PHP_FUNCTION(eval_filter_callback);
+PHP_FUNCTION(eval_blacklist_callback);
 
 static inline void sp_terminate() { zend_bailout(); }
 
diff --git a/src/snuffleupagus.c b/src/snuffleupagus.c
index 4f11e1e..cb79b23 100644
--- a/src/snuffleupagus.c
+++ b/src/snuffleupagus.c
@@ -62,7 +62,7 @@ PHP_GINIT_FUNCTION(snuffleupagus) {
 
   SP_INIT_HT(snuffleupagus_globals->disabled_functions_hook);
   SP_INIT_HT(snuffleupagus_globals->sp_internal_functions_hook);
-  SP_INIT_HT(snuffleupagus_globals->sp_eval_filter_functions_hook);
+  SP_INIT_HT(snuffleupagus_globals->sp_eval_blacklist_functions_hook);
 
   SP_INIT(snuffleupagus_globals->config.config_unserialize);
   SP_INIT(snuffleupagus_globals->config.config_random);
@@ -106,7 +106,7 @@ PHP_MSHUTDOWN_FUNCTION(snuffleupagus) {
   pefree(SNUFFLEUPAGUS_G(F), 1);
 
   FREE_HT(disabled_functions_hook);
-  FREE_HT(sp_eval_filter_functions_hook);
+  FREE_HT(sp_eval_blacklist_functions_hook);
 
 #undef FREE_HT
 
diff --git a/src/sp_config.c b/src/sp_config.c
index dc5aae9..4037e26 100644
--- a/src/sp_config.c
+++ b/src/sp_config.c
@@ -19,7 +19,8 @@ sp_config_tokens const sp_func[] = {
     {.func = parse_global, .token = SP_TOKEN_GLOBAL},
     {.func = parse_auto_cookie_secure, .token = SP_TOKEN_AUTO_COOKIE_SECURE},
     {.func = parse_disable_xxe, .token = SP_TOKEN_DISABLE_XXE},
-    {.func = parse_eval_filter, .token = SP_TOKEN_EVAL},
+    {.func = parse_eval_blacklist, .token = SP_TOKEN_EVAL_BLACKLIST},
+    {.func = parse_eval_whitelist, .token = SP_TOKEN_EVAL_WHITELIST},
     {NULL, NULL}};
 
 /* Top level keyword parsing */
diff --git a/src/sp_config.h b/src/sp_config.h
index a4a4f10..25963f1 100644
--- a/src/sp_config.h
+++ b/src/sp_config.h
@@ -177,7 +177,8 @@ typedef struct {
 #define SP_TOKEN_UNSERIALIZE_HMAC ".unserialize_hmac"
 #define SP_TOKEN_UPLOAD_VALIDATION ".upload_validation"
 #define SP_TOKEN_DISABLE_XXE ".disable_xxe"
-#define SP_TOKEN_EVAL ".eval_filter"
+#define SP_TOKEN_EVAL_BLACKLIST ".eval_blacklist"
+#define SP_TOKEN_EVAL_WHITELIST ".eval_whitelist"
 
 // common tokens
 #define SP_TOKEN_ENABLE ".enable("
@@ -231,8 +232,7 @@ typedef struct {
 #define SP_TOKEN_UPLOAD_SCRIPT ".script("
 
 // eval blacklist
-#define SP_TOKEN_EVAL_BLACKLIST ".blacklist("
-#define SP_TOKEN_EVAL_WHITELIST ".whitelist("
+#define SP_TOKEN_EVAL_LIST ".list("
 
 int sp_parse_config(const char *);
 int parse_array(sp_disabled_function *);
diff --git a/src/sp_config_keywords.c b/src/sp_config_keywords.c
index 85e04ab..c5cc950 100644
--- a/src/sp_config_keywords.c
+++ b/src/sp_config_keywords.c
@@ -102,11 +102,11 @@ int parse_global(char *line) {
   return parse_keywords(sp_config_funcs_global, line);
 }
 
-int parse_eval_filter(char *line) {
+static int parse_eval_filter_conf(char *line, sp_list_node *list) {
   char *token;
   char *rest;
   sp_config_functions sp_config_funcs[] = {
-      {parse_str, SP_TOKEN_EVAL_BLACKLIST, &rest},
+      {parse_str, SP_TOKEN_EVAL_LIST, &rest},
       {parse_empty, SP_TOKEN_SIMULATION,
        &(SNUFFLEUPAGUS_G(config).config_eval->simulation)},
       {0}};
@@ -116,11 +116,21 @@ int parse_eval_filter(char *line) {
   }
 
   while ((token = strtok_r(rest, ",", &rest))) {
-    sp_list_insert(SNUFFLEUPAGUS_G(config).config_eval->blacklist, token);
+    sp_list_insert(list, token);
   }
   return SUCCESS;
 }
 
+int parse_eval_blacklist(char *line) {
+  return parse_eval_filter_conf(line,
+                                SNUFFLEUPAGUS_G(config).config_eval->blacklist);
+}
+
+int parse_eval_whitelist(char *line) {
+  return parse_eval_filter_conf(line,
+                                SNUFFLEUPAGUS_G(config).config_eval->whitelist);
+}
+
 int parse_cookie(char *line) {
   int ret = 0;
   char *samesite = NULL;
diff --git a/src/sp_config_keywords.h b/src/sp_config_keywords.h
index d7ea36a..4205dac 100644
--- a/src/sp_config_keywords.h
+++ b/src/sp_config_keywords.h
@@ -12,6 +12,7 @@ int parse_unserialize(char *line);
 int parse_readonly_exec(char *line);
 int parse_disabled_functions(char *line);
 int parse_upload_validation(char *line);
-int parse_eval_filter(char *line);
+int parse_eval_blacklist(char *line);
+int parse_eval_whitelist(char *line);
 
 #endif  // __SP_CONFIG_KEYWORDS_H
diff --git a/src/sp_disabled_functions.c b/src/sp_disabled_functions.c
index 8e96085..fa9d625 100644
--- a/src/sp_disabled_functions.c
+++ b/src/sp_disabled_functions.c
@@ -5,7 +5,8 @@
 
 ZEND_DECLARE_MODULE_GLOBALS(snuffleupagus)
 
-static char* get_complete_function_path(
+
+char* get_complete_function_path(
     zend_execute_data const* const execute_data) {
   if (zend_is_executing() && !EG(current_execute_data)->func) {
     return NULL;
@@ -107,6 +108,7 @@ static const sp_list_node* get_config_node(const char* builtin_name) {
     return SNUFFLEUPAGUS_G(config)
         .config_disabled_constructs->construct_include;
   }
+  ZEND_ASSUME(0);
   return NULL;  // This should never happen.
 }
 
@@ -463,7 +465,7 @@ static int hook_functions(const sp_list_node* config) {
   return SUCCESS;
 }
 
-ZEND_FUNCTION(eval_filter_callback) {
+ZEND_FUNCTION(eval_blacklist_callback) {
   void (*orig_handler)(INTERNAL_FUNCTION_PARAMETERS);
   const char* current_function_name = get_active_function_name(TSRMLS_C);
 
@@ -483,7 +485,7 @@ ZEND_FUNCTION(eval_filter_callback) {
   }
 
   orig_handler = zend_hash_str_find_ptr(
-      SNUFFLEUPAGUS_G(sp_eval_filter_functions_hook), current_function_name,
+      SNUFFLEUPAGUS_G(sp_eval_blacklist_functions_hook), current_function_name,
       strlen(current_function_name));
   orig_handler(INTERNAL_FUNCTION_PARAM_PASSTHRU);
 }
@@ -503,8 +505,8 @@ int hook_disabled_functions(void) {
 
     while (it) {
       hook_function((char*)it->data,
-                    SNUFFLEUPAGUS_G(sp_eval_filter_functions_hook),
-                    PHP_FN(eval_filter_callback), false);
+                    SNUFFLEUPAGUS_G(sp_eval_blacklist_functions_hook),
+                    PHP_FN(eval_blacklist_callback), false);
       it = it->next;
     }
   }
diff --git a/src/sp_disabled_functions.h b/src/sp_disabled_functions.h
index f80c9c2..9629308 100644
--- a/src/sp_disabled_functions.h
+++ b/src/sp_disabled_functions.h
@@ -5,5 +5,6 @@ int hook_disabled_functions();
 bool should_disable(zend_execute_data *, const char *, const char *,
                     const char *);
 bool should_drop_on_ret(zval *, const zend_execute_data *const);
+char* get_complete_function_path(zend_execute_data const* const execute_data);
 
 #endif /* __SP_DISABLE_FUNCTIONS_H */
diff --git a/src/sp_execute.c b/src/sp_execute.c
index 3ce6643..c1d68d7 100644
--- a/src/sp_execute.c
+++ b/src/sp_execute.c
@@ -6,6 +6,8 @@
 ZEND_DECLARE_MODULE_GLOBALS(snuffleupagus)
 
 static void (*orig_execute_ex)(zend_execute_data *execute_data);
+static void (*orig_zend_execute_internal)(zend_execute_data *execute_data,
+                                          zval *return_value);
 static int (*orig_zend_stream_open)(const char *filename,
                                     zend_file_handle *handle);
 
@@ -29,9 +31,9 @@ ZEND_COLD static inline void terminate_if_writable(const char *filename) {
 }
 
 static void is_builtin_matching(const char *restrict const filename,
-                                char *restrict function_name,
-                                char *restrict param_name,
-                                sp_list_node *config) {
+                                const char *restrict const function_name,
+                                const char *restrict const param_name,
+                                const sp_list_node *config) {
   if (!config || !config->data) {
     return;
   }
@@ -42,6 +44,43 @@ static void is_builtin_matching(const char *restrict const filename,
   }
 }
 
+static void is_in_eval_and_whitelisted(zend_execute_data *execute_data) {
+  if (EXPECTED(0 == SNUFFLEUPAGUS_G(in_eval))) {
+    return;
+  }
+
+  if (EXPECTED(NULL == SNUFFLEUPAGUS_G(config).config_eval->whitelist->data)) {
+    return;
+  }
+
+  if (zend_is_executing() && !EG(current_execute_data)->func) {
+    return;
+  }
+
+  if (!(execute_data->func->common.function_name)) {
+    return;
+  }
+
+  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;
+    while (it) {
+      if (0 == strcmp(current_function, (char *)(it->data))) {
+        /* We've got a match, the function is whiteslited. */
+        return;
+      }
+      it = it->next;
+    }
+
+    sp_log_msg(
+        "Eval_whitelist", SP_LOG_DROP,
+        "The function '%s' isn't in the eval whitelist, dropping its call.",
+        current_function);
+    sp_terminate();
+  }
+}
+
 /* This function gets the filename in which `eval()` is called from,
  * since it looks like "foo.php(1) : eval()'d code", so we're starting
  * from the end of the string until the second closing parenthesis. */
@@ -67,28 +106,42 @@ static void sp_execute_ex(zend_execute_data *execute_data) {
     sp_terminate();
   }
 
-  if (execute_data->func->op_array.type == ZEND_EVAL_CODE) {
+  if (EX(func)->op_array.type == ZEND_EVAL_CODE) {
     SNUFFLEUPAGUS_G(in_eval)++;
-    sp_list_node *config =
+    const sp_list_node *config =
         SNUFFLEUPAGUS_G(config).config_disabled_constructs->construct_eval;
     char *filename = get_eval_filename((char *)zend_get_executed_filename());
     is_builtin_matching(filename, "eval", NULL, config);
     efree(filename);
   }
 
-  if (NULL != execute_data->func->op_array.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(execute_data->func->op_array.filename));
+      terminate_if_writable(ZSTR_VAL(EX(func)->op_array.filename));
     }
   }
 
   orig_execute_ex(execute_data);
 
-  if (true == should_drop_on_ret(execute_data->return_value, execute_data)) {
+  if (true == should_drop_on_ret(EX(return_value), execute_data)) {
     sp_terminate();
   }
 
-  SNUFFLEUPAGUS_G(in_eval)--;
+  if (ZEND_EVAL_CODE == EX(func)->op_array.type) {
+    SNUFFLEUPAGUS_G(in_eval)--;
+  }
+}
+
+static void sp_zend_execute_internal(INTERNAL_FUNCTION_PARAMETERS) {
+  is_in_eval_and_whitelisted(execute_data);
+
+  EX(func)->internal_function.handler(INTERNAL_FUNCTION_PARAM_PASSTHRU);
+
+  if (UNEXPECTED(NULL != orig_zend_execute_internal)) {
+    orig_zend_execute_internal(INTERNAL_FUNCTION_PARAM_PASSTHRU);
+  }
 }
 
 static int sp_stream_open(const char *filename, zend_file_handle *handle) {
@@ -121,11 +174,7 @@ static int sp_stream_open(const char *filename, zend_file_handle *handle) {
           is_builtin_matching(filename, "include_once", "inclusion path",
                               config);
           break;
-        case ZEND_EVAL:
-          is_builtin_matching(filename, "eval", NULL, config);
-          break;
-        default:
-          break;
+        EMPTY_SWITCH_DEFAULT_CASE();
       }
   }
 
@@ -136,16 +185,17 @@ end:
 int hook_execute(void) {
   TSRMLS_FETCH();
 
-  /* zend_execute_ex is used for "classic" function calls */
+  /* zend_execute_ex is used for "user" function calls */
   orig_execute_ex = zend_execute_ex;
   zend_execute_ex = sp_execute_ex;
 
-  /* zend_stream_open_function is used FIXME */
+  /* zend_execute_internal is used for "builtin" functions calls */
+  orig_zend_execute_internal = zend_execute_internal;
+  zend_execute_internal = sp_zend_execute_internal;
+
+  /* zend_stream_open_function is used for include-related stuff */
   orig_zend_stream_open = zend_stream_open_function;
   zend_stream_open_function = sp_stream_open;
 
-  /* zend_execute_internal is used for "indirect" functions call,
-   * like array_map or call_user_func. */
-
   return SUCCESS;
 }
diff --git a/src/tests/config/eval_backlist.ini b/src/tests/config/eval_backlist.ini
index 1e34b5b..b181598 100644
--- a/src/tests/config/eval_backlist.ini
+++ b/src/tests/config/eval_backlist.ini
@@ -1 +1 @@
-sp.eval_filter.blacklist("strlen");
+sp.eval_blacklist.list("strlen");
diff --git a/src/tests/config/eval_backlist_list.ini b/src/tests/config/eval_backlist_list.ini
index da5650d..b395d03 100644
--- a/src/tests/config/eval_backlist_list.ini
+++ b/src/tests/config/eval_backlist_list.ini
@@ -1 +1 @@
-sp.eval_filter.blacklist("strcmp,strlen");
+sp.eval_blacklist.list("strcmp,strlen");
diff --git a/src/tests/config/eval_backlist_simulation.ini b/src/tests/config/eval_backlist_simulation.ini
index fafebd5..2d8dc73 100644
--- a/src/tests/config/eval_backlist_simulation.ini
+++ b/src/tests/config/eval_backlist_simulation.ini
@@ -1 +1 @@
-sp.eval_filter.blacklist("strlen").simulation();
+sp.eval_blacklist.list("strlen").simulation();
diff --git a/src/tests/config/eval_whitelist.ini b/src/tests/config/eval_whitelist.ini
new file mode 100644
index 0000000..7a8f6ef
--- /dev/null
+++ b/src/tests/config/eval_whitelist.ini
@@ -0,0 +1 @@
+sp.eval_whitelist.list("my_fun,cos");
diff --git a/src/tests/eval_backlist_whitelist.phpt b/src/tests/eval_backlist_whitelist.phpt
new file mode 100644
index 0000000..1611288
--- /dev/null
+++ b/src/tests/eval_backlist_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.
diff --git a/src/tests/eval_whitelist_builtin.phpt b/src/tests/eval_whitelist_builtin.phpt
new file mode 100644
index 0000000..bd7c2ac
--- /dev/null
+++ b/src/tests/eval_whitelist_builtin.phpt
@@ -0,0 +1,19 @@
+--TEST--
+Eval whitelist - builtin function
+--SKIPIF--
+<?php if (!extension_loaded("snuffleupagus")) die "skip"; ?>
+--INI--
+sp.configuration_file={PWD}/config/eval_whitelist.ini
+--FILE--
+<?php 
+$a = cos(1);
+echo "Outside of eval: $a\n";
+eval('$a = cos(5);');
+echo "After allowed eval: $a\n";
+eval('$a = sin(4);');
+echo "After eval: $a\n";
+?>
+--EXPECTF--
+Outside of eval: 0.54030230586814
+After allowed eval: 0.28366218546323
+[snuffleupagus][0.0.0.0][Eval_whitelist][drop] The function 'sin' isn't in the eval whitelist, dropping its call.
diff --git a/src/tests/eval_whitelist_include_then_user.phpt b/src/tests/eval_whitelist_include_then_user.phpt
new file mode 100644
index 0000000..6d4e36a
--- /dev/null
+++ b/src/tests/eval_whitelist_include_then_user.phpt
@@ -0,0 +1,29 @@
+--TEST--
+Eval whitelist - builtin function
+--SKIPIF--
+<?php if (!extension_loaded("snuffleupagus")) die "skip"; ?>
+--INI--
+sp.configuration_file={PWD}/config/eval_whitelist.ini
+--FILE--
+<?php 
+$b = 1337;
+$dir = __DIR__;
+
+file_put_contents($dir . '/test.bla', '<?php $b = sin(1) ?>');
+
+$a = cos(1);
+echo "Outside of eval: $a\n";
+eval('$a = cos(5);');
+echo "After allowed eval: $a\n";
+eval("include_once('$dir' . '/test.bla');");
+echo "After eval: $b\n";
+?>
+--CLEAN--
+<?php
+$dir = __DIR__;
+unlink($dir . '/test.bla');
+?>
+--EXPECTF--
+Outside of eval: 0.54030230586814
+After allowed eval: 0.28366218546323
+[snuffleupagus][0.0.0.0][Eval_whitelist][drop] The function 'sin' isn't in the eval whitelist, dropping its call.
diff --git a/src/tests/eval_whitelist_user_then_builtin.phpt b/src/tests/eval_whitelist_user_then_builtin.phpt
new file mode 100644
index 0000000..8db36fc
--- /dev/null
+++ b/src/tests/eval_whitelist_user_then_builtin.phpt
@@ -0,0 +1,23 @@
+--TEST--
+Eval whitelist - builtin function
+--SKIPIF--
+<?php if (!extension_loaded("snuffleupagus")) die "skip"; ?>
+--INI--
+sp.configuration_file={PWD}/config/eval_whitelist.ini
+--FILE--
+<?php 
+
+function my_fun() {
+        return sin(10);
+}
+
+$a = my_fun(1);
+echo "Outside of eval: $a\n";
+eval('$a = my_fun(5);');
+echo "After allowed eval: $a\n";
+eval('$a = my_fun(4);');
+echo "After eval: $a\n";
+?>
+--EXPECTF--
+Outside of eval: -0.54402111088937
+[snuffleupagus][0.0.0.0][Eval_whitelist][drop] The function 'sin' isn't in the eval whitelist, dropping its call.