Refactor bailing out strategy

DataDog · Jan 7, 2025 · 60b486e · 60b486e
1 parent 22bc61b
commit 60b486e
Show file tree

Hide file tree

Showing 6 changed files with 76 additions and 7 deletions.
diff --git a/appsec/tests/extension/push_params_block_02.phpt b/appsec/tests/extension/push_params_block_02.phpt
@@ -35,7 +35,6 @@ $integration = new SomeIntegration();
 $integration->init();
 var_dump(ltrim("     Calling wrapped function"));
 var_dump("THIS SHOULD NOT GET IN THE OUTPUT");
-
 ?>
 --EXPECTHEADERS--
 Status: 404 Not Found

diff --git a/appsec/tests/extension/push_params_block_03.phpt b/appsec/tests/extension/push_params_block_03.phpt
@@ -0,0 +1,43 @@
+--TEST--
+Push address gets blocked even when within a hook
+--INI--
+extension=ddtrace.so
+datadog.appsec.enabled=1
+--FILE--
+<?php
+use function datadog\appsec\testing\{rinit,rshutdown};
+use function datadog\appsec\push_address;
+
+include __DIR__ . '/inc/mock_helper.php';
+
+$helper = Helper::createInitedRun([
+    response_list(response_request_init([[['ok', []]]])),
+    response_list(response_request_exec([[['block', ['status_code' => '404', 'type' => 'html']]], ['{"found":"attack"}','{"another":"attack"}']])),
+]);
+rinit();
+
+class SomeIntegration {
+    public function init()
+    {
+        DDTrace\install_hook("ltrim", self::hooked_function(), null);
+    }
+
+    private static function hooked_function()
+    {
+        return static function (DDTrace\HookData $hook) {
+              push_address("server.request.path_params", ["some" => "params", "more" => "parameters"]);
+              var_dump("This should be executed");
+        };
+    }
+}
+
+$integration = new SomeIntegration();
+$integration->init();
+echo "Something here to force partially booking";
+var_dump(ltrim("     Calling wrapped function"));
+var_dump("THIS SHOULD NOT GET IN THE OUTPUT");
+?>
+--EXPECTHEADERS--
+Content-type: text/html; charset=UTF-8
+--EXPECTF--
+Something here to force partially booking
diff --git a/zend_abstract_interface/sandbox/php7/sandbox.c b/zend_abstract_interface/sandbox/php7/sandbox.c
@@ -6,6 +6,7 @@ extern inline void zai_sandbox_open(zai_sandbox *sandbox);
 extern inline void zai_sandbox_close(zai_sandbox *sandbox);
 extern inline void zai_sandbox_bailout(zai_sandbox *sandbox);
 extern inline bool zai_sandbox_timed_out(void);
+extern inline bool zai_is_request_blocked(void);
 
 extern inline void zai_sandbox_error_state_backup(zai_error_state *es);
 extern inline void zai_sandbox_error_state_restore(zai_error_state *es);

diff --git a/zend_abstract_interface/sandbox/php8/sandbox.c b/zend_abstract_interface/sandbox/php8/sandbox.c
@@ -7,6 +7,7 @@ extern inline void zai_sandbox_open(zai_sandbox *sandbox);
 extern inline void zai_sandbox_close(zai_sandbox *sandbox);
 extern inline void zai_sandbox_bailout(zai_sandbox *sandbox);
 extern inline bool zai_sandbox_timed_out(void);
+extern inline bool zai_is_request_blocked(void);
 
 extern inline void zai_sandbox_error_state_backup(zai_error_state *es);
 

diff --git a/zend_abstract_interface/sandbox/sandbox.h b/zend_abstract_interface/sandbox/sandbox.h
@@ -76,7 +76,9 @@
  *
  * This function should be invoked when a bailout has been caught.
  *
- * It will restore engine state and continue in all but the case of a timeout
+ * It will restore engine state and continue in all cases except:
+ * - Timeout
+ * - Request is blocked by the extension
  */
 
 /* ######### Timeout ##########
@@ -218,8 +220,21 @@ inline bool zai_sandbox_timed_out(void) {
     return false;
 }
 
+inline bool zai_is_request_blocked(void)
+{
+    if (!PG(last_error_message)) {
+        return false;
+    }
+
+    if (strstr(ZSTR_VAL(PG(last_error_message)), "Datadog blocked the request") != NULL) {
+        return true;
+    }
+
+    return false;
+}
+
 inline void zai_sandbox_bailout(zai_sandbox *sandbox) {
-    if (!zai_sandbox_timed_out()) {
+    if (!zai_sandbox_timed_out() && !zai_is_request_blocked()) {
         zai_sandbox_engine_state_restore(&sandbox->engine_state);
 
         return;
@@ -356,8 +371,21 @@ inline bool zai_sandbox_timed_out(void) {
     return false;
 }
 
+inline bool zai_is_request_blocked(void)
+{
+    if (!PG(last_error_message)) {
+        return false;
+    }
+
+    if (strstr(PG(last_error_message), "Datadog blocked the request") != NULL) {
+        return true;
+    }
+
+    return false;
+}
+
 inline void zai_sandbox_bailout(zai_sandbox *sandbox) {
-    if (!zai_sandbox_timed_out()) {
+    if (!zai_sandbox_timed_out() && !zai_is_request_blocked()) {
         zai_sandbox_engine_state_restore(&sandbox->engine_state);
 
         return;

diff --git a/zend_abstract_interface/symbols/call.c b/zend_abstract_interface/symbols/call.c
@@ -103,9 +103,6 @@ static inline int zai_symbol_try_call(zend_fcall_info *fci, zend_fcall_info_cach
         ret = zend_call_function(fci, fcc);
     } zend_catch {
         ret = 2;
-        if (PG(last_error_message) && strstr(ZSTR_VAL(PG(last_error_message)), "Datadog blocked the request")) {
-            zend_bailout();
-        }
     } zend_end_try();
     return ret;
 }