From 4b5afd0148cef6c845a37aff68e1fbac8f5653d7 Mon Sep 17 00:00:00 2001
From: Ben Fuhrmannek
Date: Thu, 6 Jan 2022 21:22:50 +0100
Subject: prevent double checks and fixed segfault on return value access

---
 src/snuffleupagus.c         |  3 ++-
 src/sp_disabled_functions.c | 16 ++++++++++------
 src/sp_execute.c            | 19 ++++++++++++-------
 3 files changed, 24 insertions(+), 14 deletions(-)

(limited to 'src')

diff --git a/src/snuffleupagus.c b/src/snuffleupagus.c
index 1ccc412..53db721 100644
--- a/src/snuffleupagus.c
+++ b/src/snuffleupagus.c
@@ -334,7 +334,6 @@ static PHP_INI_MH(OnUpdateConfiguration) {
     }
   }
 
-  hook_disabled_functions();
   hook_execute();
   hook_cookies();
 
@@ -353,6 +352,8 @@ static PHP_INI_MH(OnUpdateConfiguration) {
     CG(compiler_options) |= ZEND_COMPILE_HANDLE_OP_ARRAY;
   }
 
+  hook_disabled_functions();
+
   // If `zend_write_default` is not NULL it is already hooked.
   if ((zend_hash_str_find(SPCFG(disabled_functions_hooked), ZEND_STRL("echo")) ||
        zend_hash_str_find(SPCFG(disabled_functions_ret_hooked), ZEND_STRL("echo"))) &&
diff --git a/src/sp_disabled_functions.c b/src/sp_disabled_functions.c
index 1d9c6c7..c0c642b 100644
--- a/src/sp_disabled_functions.c
+++ b/src/sp_disabled_functions.c
@@ -400,7 +400,6 @@ static void should_drop_on_ret(const zval* return_value,
   bool match_type = false, match_value = false;
 
   while (config) {
-    const zend_string* ret_value_str = NULL;
     sp_disabled_function const* const config_node =
         (sp_disabled_function*)(config->data);
 
@@ -444,13 +443,18 @@ static void should_drop_on_ret(const zval* return_value,
       }
     }
 
-    ret_value_str = sp_zval_to_zend_string(return_value);
+    const zend_string* ret_value_str = NULL;
+    sp_php_type ret_type = SP_PHP_TYPE_NULL;
+
+    if (return_value) {
+      ret_value_str = sp_zval_to_zend_string(return_value);
+      ret_type = Z_TYPE_P(return_value);
+    }
 
     match_type = (config_node->ret_type) &&
-                 (config_node->ret_type == Z_TYPE_P(return_value));
-    match_value = (config_node->ret || config_node->r_ret) &&
-                  (true == sp_match_value(ret_value_str, config_node->ret,
-                                          config_node->r_ret));
+                 (config_node->ret_type == ret_type);
+    match_value = return_value && (config_node->ret || config_node->r_ret) &&
+                  (true == sp_match_value(ret_value_str, config_node->ret, config_node->r_ret));
 
     if (true == match_type || true == match_value) {
       if (true == config_node->allow) {
diff --git a/src/sp_execute.c b/src/sp_execute.c
index 21a68dd..aadd145 100644
--- a/src/sp_execute.c
+++ b/src/sp_execute.c
@@ -161,14 +161,18 @@ static inline void sp_execute_handler(INTERNAL_FUNCTION_PARAMETERS, bool interna
     return;
   }
 
-  const sp_list_node *config_disabled_functions_reg = SPCFG(disabled_functions_reg).disabled_functions;
+  bool is_hooked = (zend_hash_str_find(SPG(disabled_functions_hook), VAR_AND_LEN(function_name)) || zend_hash_str_find(SPG(disabled_functions_hook), VAR_AND_LEN(function_name)));
+  if (is_hooked) {
+    sp_call_orig_execute(INTERNAL_FUNCTION_PARAM_PASSTHRU, internal);
+    return;
+  }
 
   // If we're at an internal function
   if (!execute_data->prev_execute_data ||
       !execute_data->prev_execute_data->func ||
       !ZEND_USER_CODE(execute_data->prev_execute_data->func->type) ||
       !execute_data->prev_execute_data->opline) {
-    should_disable_ht(execute_data, function_name, NULL, NULL, config_disabled_functions_reg, SPCFG(disabled_functions));
+    should_disable_ht(execute_data, function_name, NULL, NULL, SPCFG(disabled_functions_reg).disabled_functions, SPCFG(disabled_functions));
   } else {  // If we're at a userland function call
     switch (execute_data->prev_execute_data->opline->opcode) {
       case ZEND_DO_FCALL:
@@ -176,7 +180,7 @@ static inline void sp_execute_handler(INTERNAL_FUNCTION_PARAMETERS, bool interna
       case ZEND_DO_ICALL:
       case ZEND_DO_UCALL:
       case ZEND_TICKS:
-        should_disable_ht(execute_data, function_name, NULL, NULL, config_disabled_functions_reg, SPCFG(disabled_functions));
+        should_disable_ht(execute_data, function_name, NULL, NULL, SPCFG(disabled_functions_reg).disabled_functions, SPCFG(disabled_functions));
       default:
         break;
     }
@@ -188,23 +192,24 @@ static inline void sp_execute_handler(INTERNAL_FUNCTION_PARAMETERS, bool interna
   zval ret_val;
   if (EX(return_value) == NULL) {
     memset(&ret_val, 0, sizeof(ret_val));
-    EX(return_value) = &ret_val;
+    return_value = EX(return_value) = &ret_val;
   }
 
   sp_call_orig_execute(INTERNAL_FUNCTION_PARAM_PASSTHRU, internal);
 
-  should_drop_on_ret_ht(EX(return_value), function_name, SPCFG(disabled_functions_reg_ret).disabled_functions, SPCFG(disabled_functions_ret), execute_data);
+  should_drop_on_ret_ht(return_value, function_name, SPCFG(disabled_functions_reg_ret).disabled_functions, SPCFG(disabled_functions_ret), execute_data);
+
   efree(function_name);
 
   if (EX(return_value) == &ret_val) {
-    EX(return_value) = NULL;
+    return_value = EX(return_value) = NULL;
   }
 
 }
 
 
 static void sp_execute_ex(zend_execute_data *execute_data) {
-  sp_execute_handler(execute_data, NULL, false);
+  sp_execute_handler(execute_data, execute_data ? EX(return_value) : NULL, false);
 }
 
 static void sp_zend_execute_internal(INTERNAL_FUNCTION_PARAMETERS) {
-- 
cgit v1.3