Skip to content

Commit

Permalink
Refactor bailing out strategy
Browse files Browse the repository at this point in the history
  • Loading branch information
estringana committed Dec 12, 2024
1 parent d393b43 commit ee5bff3
Show file tree
Hide file tree
Showing 6 changed files with 84 additions and 7 deletions.
1 change: 0 additions & 1 deletion appsec/tests/extension/push_params_block_02.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
43 changes: 43 additions & 0 deletions appsec/tests/extension/push_params_block_03.phpt
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions zend_abstract_interface/sandbox/php7/sandbox.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions zend_abstract_interface/sandbox/php8/sandbox.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
42 changes: 39 additions & 3 deletions zend_abstract_interface/sandbox/sandbox.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 ##########
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
3 changes: 0 additions & 3 deletions zend_abstract_interface/symbols/call.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down

0 comments on commit ee5bff3

Please sign in to comment.