Skip to content

Commit 4a6498d

Browse files
estringanaestringana
authored
Blocking from a hook is not stopping code execution (#2836)
1 parent 906cbc5 commit 4a6498d

File tree

7 files changed

+161
-3
lines changed

7 files changed

+161
-3
lines changed

‎appsec/tests/extension/push_params_block_02.phpt

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
--TEST--
2+
Push address gets blocked even when within a hook
3+
--INI--
4+
extension=ddtrace.so
5+
datadog.appsec.enabled=1
6+
--FILE--
7+
<?php
8+
use function datadog\appsec\testing\{rinit,rshutdown};
9+
use function datadog\appsec\push_address;
10+
11+
include __DIR__ . '/inc/mock_helper.php';
12+
13+
$helper = Helper::createInitedRun([
14+
response_list(response_request_init([[['ok', []]]])),
15+
response_list(response_request_exec([[['block', ['status_code' => '404', 'type' => 'html']]], ['{"found":"attack"}','{"another":"attack"}']])),
16+
]);
17+
rinit();
18+
19+
class SomeIntegration {
20+
public function init()
21+
{
22+
DDTrace\install_hook("ltrim", self::hooked_function(), null);
23+
}
24+
25+
private static function hooked_function()
26+
{
27+
return static function (DDTrace\HookData $hook) {
28+
push_address("server.request.path_params", ["some" => "params", "more" => "parameters"]);
29+
var_dump("This should be executed");
30+
};
31+
}
32+
}
33+
34+
$integration = new SomeIntegration();
35+
$integration->init();
36+
var_dump(ltrim(" Calling wrapped function"));
37+
var_dump("THIS SHOULD NOT GET IN THE OUTPUT");
38+
?>
39+
--EXPECTHEADERS--
40+
Status: 404 Not Found
41+
Content-type: text/html;charset=UTF-8
42+
--EXPECTF--
43+
<!DOCTYPE html><html lang="en"><head><meta charset="UTF-8"><meta name="viewport" content="width=device-width,initial-scale=1"><title>You've been blocked</title><style>a,body,div,html,span{margin:0;padding:0;border:0;font-size:100%;font:inherit;vertical-align:baseline}body{background:-webkit-radial-gradient(26% 19%,circle,#fff,#f4f7f9);background:radial-gradient(circle at 26% 19%,#fff,#f4f7f9);display:-webkit-box;display:-ms-flexbox;display:flex;-webkit-box-pack:center;-ms-flex-pack:center;justify-content:center;-webkit-box-align:center;-ms-flex-align:center;align-items:center;-ms-flex-line-pack:center;align-content:center;width:100%;min-height:100vh;line-height:1;flex-direction:column}p{display:block}main{text-align:center;flex:1;display:-webkit-box;display:-ms-flexbox;display:flex;-webkit-box-pack:center;-ms-flex-pack:center;justify-content:center;-webkit-box-align:center;-ms-flex-align:center;align-items:center;-ms-flex-line-pack:center;align-content:center;flex-direction:column}p{font-size:18px;line-height:normal;color:#646464;font-family:sans-serif;font-weight:400}a{color:#4842b7}footer{width:100%;text-align:center}footer p{font-size:16px}</style></head><body><main><p>Sorry, you cannot access this page. Please contact the customer service team.</p></main><footer><p>Security provided by <a href="https://www.datadoghq.com/product/security-platform/application-security-monitoring/" target="_blank">Datadog</a></p></footer></body></html>

‎appsec/tests/extension/push_params_block_03.phpt

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
--TEST--
2+
Push address gets blocked even when within a hook
3+
--INI--
4+
extension=ddtrace.so
5+
datadog.appsec.enabled=1
6+
--FILE--
7+
<?php
8+
use function datadog\appsec\testing\{rinit,rshutdown};
9+
use function datadog\appsec\push_address;
10+
11+
include __DIR__ . '/inc/mock_helper.php';
12+
13+
$helper = Helper::createInitedRun([
14+
response_list(response_request_init([[['ok', []]]])),
15+
response_list(response_request_exec([[['block', ['status_code' => '404', 'type' => 'html']]], ['{"found":"attack"}','{"another":"attack"}']])),
16+
]);
17+
rinit();
18+
19+
class SomeIntegration {
20+
public function init()
21+
{
22+
DDTrace\install_hook("ltrim", self::hooked_function(), null);
23+
}
24+
25+
private static function hooked_function()
26+
{
27+
return static function (DDTrace\HookData $hook) {
28+
push_address("server.request.path_params", ["some" => "params", "more" => "parameters"]);
29+
var_dump("This should be executed");
30+
};
31+
}
32+
}
33+
34+
$integration = new SomeIntegration();
35+
$integration->init();
36+
echo "Something here to force partially booking";
37+
var_dump(ltrim(" Calling wrapped function"));
38+
var_dump("THIS SHOULD NOT GET IN THE OUTPUT");
39+
?>
40+
--EXPECTHEADERS--
41+
Content-type: text/html; charset=UTF-8
42+
--EXPECTF--
43+
Something here to force partially booking

‎appsec/tests/extension/push_params_redirect.phpt renamed to ‎appsec/tests/extension/push_params_redirect_01.phpt

File renamed without changes.

‎appsec/tests/extension/push_params_redirect_02.phpt

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
--TEST--
2+
Push address gets blocked
3+
--INI--
4+
extension=ddtrace.so
5+
datadog.appsec.enabled=1
6+
--FILE--
7+
<?php
8+
use function datadog\appsec\testing\{rinit,rshutdown};
9+
use function datadog\appsec\push_address;
10+
11+
include __DIR__ . '/inc/mock_helper.php';
12+
13+
$helper = Helper::createInitedRun([
14+
response_list(response_request_init([[['ok', []]]])),
15+
response_list(response_request_exec([[['redirect', ['status_code' => '303', 'location' => 'https://datadoghq.com']]], []])),
16+
]);
17+
rinit();
18+
19+
class SomeIntegration {
20+
public function init()
21+
{
22+
DDTrace\install_hook("ltrim", self::hooked_function(), null);
23+
}
24+
25+
private static function hooked_function()
26+
{
27+
return static function (DDTrace\HookData $hook) {
28+
push_address("server.request.path_params", ["some" => "params", "more" => "parameters"]);
29+
var_dump("This should be executed");
30+
};
31+
}
32+
}
33+
34+
$integration = new SomeIntegration();
35+
$integration->init();
36+
var_dump(ltrim(" Calling wrapped function"));
37+
var_dump("THIS SHOULD NOT GET IN THE OUTPUT");
38+
?>
39+
--EXPECTHEADERS--
40+
Status: 303 See Other
41+
Content-type: text/html; charset=UTF-8
42+
--EXPECTF--

‎zend_abstract_interface/sandbox/php7/sandbox.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ extern inline void zai_sandbox_open(zai_sandbox *sandbox);
66
extern inline void zai_sandbox_close(zai_sandbox *sandbox);
77
extern inline void zai_sandbox_bailout(zai_sandbox *sandbox);
88
extern inline bool zai_sandbox_timed_out(void);
9+
extern inline bool zai_is_request_blocked(void);
910

1011
extern inline void zai_sandbox_error_state_backup(zai_error_state *es);
1112
extern inline void zai_sandbox_error_state_restore(zai_error_state *es);

‎zend_abstract_interface/sandbox/php8/sandbox.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ extern inline void zai_sandbox_open(zai_sandbox *sandbox);
77
extern inline void zai_sandbox_close(zai_sandbox *sandbox);
88
extern inline void zai_sandbox_bailout(zai_sandbox *sandbox);
99
extern inline bool zai_sandbox_timed_out(void);
10+
extern inline bool zai_is_request_blocked(void);
1011

1112
extern inline void zai_sandbox_error_state_backup(zai_error_state *es);
1213

‎zend_abstract_interface/sandbox/sandbox.h

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,9 @@
7676
*
7777
* This function should be invoked when a bailout has been caught.
7878
*
79-
* It will restore engine state and continue in all but the case of a timeout
79+
* It will restore engine state and continue in all cases except:
80+
* - Timeout
81+
* - Request is blocked by the extension
8082
*/
8183

8284
/* ######### Timeout ##########
@@ -218,8 +220,21 @@ inline bool zai_sandbox_timed_out(void) {
218220
return false;
219221
}
220222

223+
inline bool zai_is_request_blocked(void)
224+
{
225+
if (!PG(last_error_message)) {
226+
return false;
227+
}
228+
229+
if (strstr(ZSTR_VAL(PG(last_error_message)), "Datadog blocked the request") != NULL) {
230+
return true;
231+
}
232+
233+
return false;
234+
}
235+
221236
inline void zai_sandbox_bailout(zai_sandbox *sandbox) {
222-
if (!zai_sandbox_timed_out()) {
237+
if (!zai_sandbox_timed_out() && !zai_is_request_blocked()) {
223238
zai_sandbox_engine_state_restore(&sandbox->engine_state);
224239

225240
return;
@@ -356,8 +371,21 @@ inline bool zai_sandbox_timed_out(void) {
356371
return false;
357372
}
358373

374+
inline bool zai_is_request_blocked(void)
375+
{
376+
if (!PG(last_error_message)) {
377+
return false;
378+
}
379+
380+
if (strstr(PG(last_error_message), "Datadog blocked the request") != NULL) {
381+
return true;
382+
}
383+
384+
return false;
385+
}
386+
359387
inline void zai_sandbox_bailout(zai_sandbox *sandbox) {
360-
if (!zai_sandbox_timed_out()) {
388+
if (!zai_sandbox_timed_out() && !zai_is_request_blocked()) {
361389
zai_sandbox_engine_state_restore(&sandbox->engine_state);
362390

363391
return;

0 commit comments

Comments
 (0)