Skip to content

Commit 0627472

Browse files
estringanaestringana
committed
Report rasp rule used on wrapped functions
1 parent 6300ec5 commit 0627472

18 files changed

+42
-55
lines changed

‎appsec/src/extension/commands/request_exec.c

+4-3
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515

1616
struct ctx {
1717
struct req_info req_info; // dd_command_proc_resp_verd_span_data expect it
18-
dd_rasp_rule rasp_rule;
18+
zend_string *nullable rasp_rule;
1919
zval *nonnull data;
2020
};
2121

@@ -30,7 +30,8 @@ static const dd_command_spec _spec = {
3030
.config_features_cb = dd_command_process_config_features_unexpected,
3131
};
3232

33-
dd_result dd_request_exec(dd_conn *nonnull conn, zval *nonnull data, unsigned rasp_rule)
33+
dd_result dd_request_exec(
34+
dd_conn *nonnull conn, zval *nonnull data, zend_string *nullable rasp_rule)
3435
{
3536
if (Z_TYPE_P(data) != IS_ARRAY) {
3637
mlog(dd_log_debug, "Invalid data provided to command request_exec, "
@@ -48,7 +49,7 @@ static dd_result _pack_command(mpack_writer_t *nonnull w, void *nonnull _ctx)
4849
assert(_ctx != NULL);
4950
struct ctx *ctx = _ctx;
5051

51-
mpack_write(w, ctx->rasp_rule);
52+
dd_mpack_write_nullable_zstr(w, ctx->rasp_rule);
5253
dd_mpack_write_zval(w, ctx->data);
5354

5455
return dd_success;

‎appsec/src/extension/commands/request_exec.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,5 @@
99
#include <SAPI.h>
1010
#include <php.h>
1111

12-
dd_result dd_request_exec(dd_conn *nonnull conn, zval *nonnull data, unsigned rasp_rule);
12+
dd_result dd_request_exec(
13+
dd_conn *nonnull conn, zval *nonnull data, zend_string *nullable rasp_rule);

‎appsec/src/extension/ddappsec.c

+4-18
Original file line numberDiff line numberDiff line change
@@ -488,8 +488,8 @@ static PHP_FUNCTION(datadog_appsec_push_addresses)
488488
}
489489

490490
zval *addresses = NULL;
491-
long rasp_rule = dd_rasp_rule_none;
492-
if (zend_parse_parameters(ZEND_NUM_ARGS(), "z|l", &addresses, &rasp_rule) ==
491+
zend_string *rasp_rule = NULL;
492+
if (zend_parse_parameters(ZEND_NUM_ARGS(), "z|S", &addresses, &rasp_rule) ==
493493
FAILURE) {
494494
RETURN_FALSE;
495495
}
@@ -498,11 +498,7 @@ static PHP_FUNCTION(datadog_appsec_push_addresses)
498498
RETURN_FALSE;
499499
}
500500

501-
if (rasp_rule != dd_rasp_rule_lfi && rasp_rule != dd_rasp_rule_ssrf) {
502-
rasp_rule = dd_rasp_rule_none;
503-
}
504-
505-
if (rasp_rule != dd_rasp_rule_none &&
501+
if (rasp_rule && ZSTR_LEN(rasp_rule) > 0 &&
506502
!get_global_DD_APPSEC_RASP_ENABLED()) {
507503
return;
508504
}
@@ -515,7 +511,7 @@ static PHP_FUNCTION(datadog_appsec_push_addresses)
515511

516512
dd_result res = dd_request_exec(conn, addresses, rasp_rule);
517513

518-
if (rasp_rule > dd_rasp_rule_none) {
514+
if (rasp_rule && ZSTR_LEN(rasp_rule) > 0) {
519515
clock_gettime(CLOCK_MONOTONIC_RAW, &end);
520516
elapsed =
521517
((int64_t)end.tv_sec - (int64_t)start.tv_sec) *
@@ -575,16 +571,6 @@ static void _register_testing_objects()
575571
{
576572
dd_phpobj_reg_funcs(functions);
577573

578-
# define _REG_RASP_CONST(php_name, value) \
579-
do { \
580-
char v[] = "datadog\\appsec\\rasp\\" php_name; \
581-
dd_phpobj_reg_long_const( \
582-
v, sizeof(v) - 1, value, CONST_CS | CONST_PERSISTENT); \
583-
} while (0)
584-
585-
_REG_RASP_CONST("LFI", dd_rasp_rule_lfi);
586-
_REG_RASP_CONST("SSRF", dd_rasp_rule_ssrf);
587-
588574
if (!get_global_DD_APPSEC_TESTING()) {
589575
return;
590576
}

‎appsec/src/extension/ddappsec.h

-6
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,4 @@ int dd_appsec_rshutdown(bool ignore_verdict);
6767

6868
#define PHP_DDAPPSEC_EXTNAME "ddappsec"
6969

70-
typedef enum {
71-
dd_rasp_rule_none = 0,
72-
dd_rasp_rule_lfi,
73-
dd_rasp_rule_ssrf,
74-
} dd_rasp_rule;
75-
7670
#endif // DDAPPSEC_H

‎appsec/tests/extension/actions_handling_01.phpt

+1-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ array(2) {
3232
[1]=>
3333
array(2) {
3434
[0]=>
35-
int(0)
35+
string(0) ""
3636
[1]=>
3737
array(1) {
3838
["server.request.path_params"]=>

‎appsec/tests/extension/push_params_ok_01.phpt

+1-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ array(2) {
3232
[1]=>
3333
array(2) {
3434
[0]=>
35-
int(0)
35+
string(0) ""
3636
[1]=>
3737
array(1) {
3838
["server.request.path_params"]=>

‎appsec/tests/extension/push_params_ok_02.phpt

+1-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ array(2) {
3232
[1]=>
3333
array(2) {
3434
[0]=>
35-
int(0)
35+
string(0) ""
3636
[1]=>
3737
array(1) {
3838
["server.request.path_params"]=>

‎appsec/tests/extension/push_params_ok_03.phpt

+1-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ array(2) {
3232
[1]=>
3333
array(2) {
3434
[0]=>
35-
int(0)
35+
string(0) ""
3636
[1]=>
3737
array(1) {
3838
["server.request.path_params"]=>

‎appsec/tests/extension/push_params_ok_04.phpt

+2-2
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ $helper = Helper::createInitedRun([
1818
]);
1919

2020
var_dump(rinit());
21-
push_addresses(["server.request.path_params" => 1234], \datadog\appsec\rasp\LFI);
21+
push_addresses(["server.request.path_params" => 1234], "lfi");
2222
var_dump(rshutdown());
2323
print_r(root_span_get_metrics());
2424

@@ -40,7 +40,7 @@ array(2) {
4040
[1]=>
4141
array(2) {
4242
[0]=>
43-
int(1)
43+
string(3) "lfi"
4444
[1]=>
4545
array(1) {
4646
["server.request.path_params"]=>

‎appsec/tests/extension/push_params_ok_05.phpt

+1-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ $helper = Helper::createInitedRun([
1818
]);
1919

2020
var_dump(rinit());
21-
push_addresses(["server.request.path_params" => 1234], \datadog\appsec\rasp\LFI);
21+
push_addresses(["server.request.path_params" => 1234], "lfi");
2222
var_dump(rshutdown());
2323
print_r(root_span_get_metrics());
2424

‎appsec/tests/extension/push_params_ok_06.phpt

+1-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ $helper = Helper::createInitedRun([
1616
]);
1717

1818
var_dump(rinit());
19-
push_addresses(["server.request.path_params" => 1234], \datadog\appsec\rasp\LFI);
19+
push_addresses(["server.request.path_params" => 1234], "lfi");
2020
var_dump(rshutdown());
2121
print_r(root_span_get_metrics());
2222

‎appsec/tests/extension/push_params_ok_07.phpt

+1-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ array(2) {
3232
[1]=>
3333
array(2) {
3434
[0]=>
35-
int(0)
35+
string(0) ""
3636
[1]=>
3737
array(2) {
3838
["server.request.path_params"]=>

‎appsec/tests/extension/push_params_ok_08.phpt

+2-2
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ $helper = Helper::createInitedRun([
1818
]);
1919

2020
var_dump(rinit());
21-
push_addresses(["server.request.path_params" => ["some" => "params", "more" => "parameters"]], \datadog\appsec\rasp\LFI);
21+
push_addresses(["server.request.path_params" => ["some" => "params", "more" => "parameters"]], "lfi");
2222
var_dump(rshutdown());
2323

2424
var_dump($helper->get_command("request_exec"));
@@ -33,7 +33,7 @@ array(2) {
3333
[1]=>
3434
array(2) {
3535
[0]=>
36-
int(1)
36+
string(3) "lfi"
3737
[1]=>
3838
array(1) {
3939
["server.request.path_params"]=>

‎appsec/tests/extension/push_params_ok_09.phpt

+2-2
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ $helper = Helper::createInitedRun([
1818
]);
1919

2020
var_dump(rinit());
21-
push_addresses(["server.request.path_params" => ["some" => "params", "more" => "parameters"]], \datadog\appsec\rasp\SSRF);
21+
push_addresses(["server.request.path_params" => ["some" => "params", "more" => "parameters"]], "ssrf");
2222
var_dump(rshutdown());
2323

2424
var_dump($helper->get_command("request_exec"));
@@ -33,7 +33,7 @@ array(2) {
3333
[1]=>
3434
array(2) {
3535
[0]=>
36-
int(2)
36+
string(4) "ssrf"
3737
[1]=>
3838
array(1) {
3939
["server.request.path_params"]=>

‎appsec/tests/extension/request_exec.phpt

+1-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ array(2) {
4747
[1]=>
4848
array(2) {
4949
[0]=>
50-
int(0)
50+
string(0) ""
5151
[1]=>
5252
array(3) {
5353
["key 01"]=>

‎src/DDTrace/Integrations/Filesystem/FilesystemIntegration.php

+4-1
Original file line numberDiff line numberDiff line change
@@ -60,21 +60,24 @@ private static function preHook($variant)
6060
$protocol = isset($protocol[1]) ? $protocol[1]: "";
6161

6262
$addresses = [];
63+
$rule = "";
6364

6465
if (empty($protocol) || $protocol === 'file') {
6566
$addresses["server.io.fs.file"] = $hook->args[0];
67+
$rule = "lfi";
6668
}
6769

6870
if (in_array($variant, ['file_get_contents', 'fopen']) &&
6971
in_array($protocol, ['http', 'https', 'ftp', 'ftps'])) {
7072
$addresses["server.io.net.url"] = $hook->args[0];
73+
$rule = "ssrf";
7174
}
7275

7376
if (empty($addresses)) {
7477
return;
7578
}
7679

77-
\datadog\appsec\push_addresses($addresses, true);
80+
\datadog\appsec\push_addresses($addresses, $rule);
7881
};
7982
}
8083

‎tests/Appsec/Mock.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -210,10 +210,10 @@ function track_user_signup_event($userId, $metadata)
210210
* This function is exposed by appsec but here we are mocking it for tests
211211
* @param array $params
212212
*/
213-
function push_addresses($addresses, $rasp = false) {
213+
function push_addresses($addresses, $rasp = "") {
214214
if(!appsecMockEnabled()) {
215215
return;
216216
}
217-
AppsecStatus::getInstance()->addEvent(['rasp' => $rasp, $addresses], 'push_addresses');
217+
AppsecStatus::getInstance()->addEvent(['rasp_rule' => $rasp, $addresses], 'push_addresses');
218218
}
219219
}

‎tests/Integrations/Filesystem/FilesystemTest.php

+12-10
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,15 @@ protected static function getEnvs()
2222
]);
2323
}
2424

25-
protected function assertEvent(string $value, $traces, $ssrf = false)
25+
protected function assertEvent(string $value, $traces, string $rasp_rule)
2626
{
2727
$events = AppsecStatus::getInstance()->getEvents();
2828
$this->assertEquals(1, count($events));
2929
$this->assertEquals(1, count($events[0][0]));
30-
$key = !$ssrf ? "server.io.fs.file" : "server.io.net.url";
30+
$key = $rasp_rule == "lfi" ? "server.io.fs.file" : "server.io.net.url";
3131
$this->assertEquals($value, $events[0][0][$key]);
3232
$this->assertEquals('push_addresses', $events[0]['eventName']);
33-
$this->assertTrue($events[0]['rasp']);
33+
$this->assertEquals($rasp_rule, $events[0]['rasp_rule']);
3434
}
3535

3636
public function ssrfProtocols()
@@ -54,7 +54,7 @@ public function testSsrfProtocols($protocol)
5454
TestCase::assertSame('OK', $response);
5555
});
5656

57-
$this->assertEvent($url, $traces, true);
57+
$this->assertEvent($url, $traces, "ssrf");
5858
}
5959

6060
public function testInvalidProtocol()
@@ -80,19 +80,21 @@ public function wrappedFunctions()
8080
}
8181

8282
/**
83+
* With no protocol all default to files system wrapper and therefore lfi
8384
* @dataProvider wrappedFunctions
8485
*/
8586
public function testNoProtocol($targetFunction, $ssrf)
8687
{
8788
$traces = $this->tracesFromWebRequest(function () use($targetFunction) {
8889
$response = $this->call(GetSpec::create('Root', '/?function='.$targetFunction.'&path=./somefile'));
89-
90+
//The str_replace replace is because the content of the file is sent to the output on some functions only
9091
TestCase::assertSame('OK', str_replace('some content', '', $response));
9192
});
92-
$this->assertEvent('./somefile', $traces, false);
93+
$this->assertEvent('./somefile', $traces, "lfi");
9394
}
9495

9596
/**
97+
* With file protocol always use LFI
9698
* @dataProvider wrappedFunctions
9799
*/
98100
public function testWithFileProtocol($targetFunction, $ssrf)
@@ -101,10 +103,11 @@ public function testWithFileProtocol($targetFunction, $ssrf)
101103
$response = $this->call(GetSpec::create('Root', '/?function='.$targetFunction.'&path=file://somefile'));
102104
TestCase::assertSame('OK', $response);
103105
});
104-
$this->assertEvent('file://somefile', $traces, false);
106+
$this->assertEvent('file://somefile', $traces, "lfi");
105107
}
106108

107109
/**
110+
* HTTP protocol is valid for SSRF
108111
* @dataProvider wrappedFunctions
109112
*/
110113
public function testWithHttpProtocol($targetFunction, $ssrf)
@@ -113,10 +116,9 @@ public function testWithHttpProtocol($targetFunction, $ssrf)
113116
$response = $this->call(GetSpec::create('Root', '/?function='.$targetFunction.'&path=http://some.url'));
114117
TestCase::assertSame('OK', $response);
115118
});
116-
$events = AppsecStatus::getInstance()->getEvents();
117119
if ($ssrf) {
118-
$this->assertEvent('http://some.url', $traces, $ssrf);
119-
} else { //Only lfi and non valid protocol
120+
$this->assertEvent('http://some.url', $traces, "ssrf");
121+
} else {
120122
$this->assertEquals(0, count(AppsecStatus::getInstance()->getEvents()));
121123
}
122124
}

0 commit comments

Comments
 (0)