From 2ef1f5d078de23b07680ac7ad4eb1be57def652d Mon Sep 17 00:00:00 2001 From: Alejandro Estringana Ruiz Date: Thu, 5 Sep 2024 17:18:10 +0200 Subject: [PATCH 1/3] Replicate issue on a PHPT tests --- .../tests/extension/push_params_block_02.phpt | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 appsec/tests/extension/push_params_block_02.phpt diff --git a/appsec/tests/extension/push_params_block_02.phpt b/appsec/tests/extension/push_params_block_02.phpt new file mode 100644 index 0000000000..322dabd862 --- /dev/null +++ b/appsec/tests/extension/push_params_block_02.phpt @@ -0,0 +1,45 @@ +--TEST-- +Push address gets blocked even when within a hook +--INI-- +extension=ddtrace.so +datadog.appsec.enabled=1 +--FILE-- + '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 PHP_EOL; +var_dump(ltrim(" Calling wrapped function")); +var_dump("THIS SHOULD NOT GET IN THE OUTPUT"); + +?> +--EXPECTHEADERS-- +Status: 404 Not Found +Content-type: text/html; charset=UTF-8 +--EXPECTF-- +You've been blocked

Sorry, you cannot access this page. Please contact the customer service team.

\ No newline at end of file From b434f6013dc2eb998cc4a6112bda676b7da87c73 Mon Sep 17 00:00:00 2001 From: Alejandro Estringana Ruiz Date: Fri, 29 Nov 2024 11:14:36 +0100 Subject: [PATCH 2/3] Draft of how to overcome problem --- appsec/tests/extension/push_params_block_02.phpt | 3 +-- zend_abstract_interface/symbols/call.c | 3 +++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/appsec/tests/extension/push_params_block_02.phpt b/appsec/tests/extension/push_params_block_02.phpt index 322dabd862..a1587486ba 100644 --- a/appsec/tests/extension/push_params_block_02.phpt +++ b/appsec/tests/extension/push_params_block_02.phpt @@ -33,13 +33,12 @@ class SomeIntegration { $integration = new SomeIntegration(); $integration->init(); -echo PHP_EOL; var_dump(ltrim(" Calling wrapped function")); var_dump("THIS SHOULD NOT GET IN THE OUTPUT"); ?> --EXPECTHEADERS-- Status: 404 Not Found -Content-type: text/html; charset=UTF-8 +Content-type: text/html;charset=UTF-8 --EXPECTF-- You've been blocked

Sorry, you cannot access this page. Please contact the customer service team.

\ No newline at end of file diff --git a/zend_abstract_interface/symbols/call.c b/zend_abstract_interface/symbols/call.c index 07c90843f5..c222473caf 100644 --- a/zend_abstract_interface/symbols/call.c +++ b/zend_abstract_interface/symbols/call.c @@ -103,6 +103,9 @@ 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; } From 7fd4a505ff268433b9f6a91fa28f6337bb7ad124 Mon Sep 17 00:00:00 2001 From: Alejandro Estringana Ruiz Date: Fri, 29 Nov 2024 16:11:45 +0100 Subject: [PATCH 3/3] Refactor bailing out strategy --- .../tests/extension/push_params_block_02.phpt | 1 - .../tests/extension/push_params_block_03.phpt | 43 +++++++++++++++++++ .../sandbox/php7/sandbox.c | 1 + .../sandbox/php8/sandbox.c | 1 + zend_abstract_interface/sandbox/sandbox.h | 42 ++++++++++++++++-- zend_abstract_interface/symbols/call.c | 3 -- 6 files changed, 84 insertions(+), 7 deletions(-) create mode 100644 appsec/tests/extension/push_params_block_03.phpt diff --git a/appsec/tests/extension/push_params_block_02.phpt b/appsec/tests/extension/push_params_block_02.phpt index a1587486ba..a11c052a8f 100644 --- 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 new file mode 100644 index 0000000000..eba1d26d4a --- /dev/null +++ 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-- + '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 \ No newline at end of file diff --git a/zend_abstract_interface/sandbox/php7/sandbox.c b/zend_abstract_interface/sandbox/php7/sandbox.c index dadc09afa5..e6efd13e16 100644 --- 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 index 92fc0589ea..ddd93bb2b1 100644 --- 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 index c441b5bc16..4ce65f4b9c 100644 --- 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,25 @@ inline bool zai_sandbox_timed_out(void) { return false; } +inline bool zai_is_request_blocked(void) +{ + if (!PG(last_error_message)) { + return false; + } + + if (strcmp("Datadog blocked the request and presented a static error page", ZSTR_VAL(PG(last_error_message))) == 0) { + return true; + } + + if (strcmp("Datadog blocked the request, but the response has already been partially committed", ZSTR_VAL(PG(last_error_message))) == 0) { + 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 +375,25 @@ inline bool zai_sandbox_timed_out(void) { return false; } +inline bool zai_is_request_blocked(void) +{ + if (!PG(last_error_message)) { + return false; + } + + if (strcmp("Datadog blocked the request and presented a static error page", ZSTR_VAL(PG(last_error_message))) == 0) { + return true; + } + + if (strcmp("Datadog blocked the request, but the response has already been partially committed", ZSTR_VAL(PG(last_error_message))) == 0) { + 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 index c222473caf..07c90843f5 100644 --- 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; }