Skip to content

Commit cc11b8e

Browse files
iamluciamluc
authored
Fix use-after-free with live-debugger (#2989)
* Fix user-after-free with debugger * Avoid copies * fix compilation + fix on PHP < 7.4
1 parent 4a6498d commit cc11b8e

File tree

7 files changed

+157
-4
lines changed

7 files changed

+157
-4
lines changed

‎ext/ddtrace.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1508,6 +1508,8 @@ static PHP_MSHUTDOWN_FUNCTION(ddtrace) {
15081508

15091509
ddtrace_sidecar_shutdown();
15101510

1511+
ddtrace_live_debugger_mshutdown();
1512+
15111513
#if PHP_VERSION_ID >= 80000 && PHP_VERSION_ID < 80100
15121514
// See dd_register_span_data_ce for explanation
15131515
zend_string *key;

‎ext/ddtrace.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ ZEND_BEGIN_MODULE_GLOBALS(ddtrace)
135135
ddog_RemoteConfigState *remote_config_state;
136136
ddog_AgentInfoReader *agent_info_reader;
137137
zend_arena *debugger_capture_arena;
138+
HashTable debugger_capture_ephemerals;
138139
ddog_Vec_DebuggerPayload exception_debugger_buffer;
139140
HashTable active_rc_hooks;
140141
HashTable *agent_rate_by_service;

‎ext/exception_serialize.c

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,10 @@ void ddtrace_create_capture_value(zval *zv, struct ddog_CaptureValue *value, con
230230
zend_get_properties_for(zv, ZEND_PROP_PURPOSE_DEBUG)
231231
#endif
232232
: Z_OBJPROP_P(zv);
233+
234+
if (!ht) {
235+
break;
236+
}
233237
ZEND_HASH_REVERSE_FOREACH_STR_KEY_VAL(ht, key, val) {
234238
if (!key) {
235239
continue;
@@ -263,10 +267,10 @@ void ddtrace_create_capture_value(zval *zv, struct ddog_CaptureValue *value, con
263267
if (ce->type == ZEND_INTERNAL_CLASS) {
264268
#if PHP_VERSION_ID < 70400
265269
if (is_temp) {
266-
zend_array_release(ht);
270+
zend_hash_next_index_insert_ptr(&DDTRACE_G(debugger_capture_ephemerals), ht);
267271
}
268272
#else
269-
zend_release_properties(ht);
273+
zend_hash_next_index_insert_ptr(&DDTRACE_G(debugger_capture_ephemerals), ht);
270274
#endif
271275
}
272276
break;
@@ -392,7 +396,7 @@ static void ddtrace_collect_exception_debug_data(zend_object *exception, zend_st
392396
LOG(TRACE, "Skipping exception replay capture due to hash %.*s already recently hit", hash_len, exception_hash);
393397
return;
394398
}
395-
399+
396400
char *exception_id = zend_arena_alloc(&DDTRACE_G(debugger_capture_arena), uuid_len);
397401
ddog_snapshot_format_new_uuid((uint8_t(*)[uuid_len])exception_id);
398402

‎ext/live_debugger.c

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -781,7 +781,7 @@ static const void *dd_eval_fetch_identifier(void *ctx, const ddog_CharSlice *nam
781781
}
782782
return NULL;
783783
}
784-
784+
785785
return NULL;
786786
}
787787

@@ -1294,7 +1294,19 @@ ddog_LiveDebuggerSetup ddtrace_live_debugger_setup = {
12941294
.evaluator = &dd_evaluator,
12951295
};
12961296

1297+
static void dd_ht_ephemerals_dtor(void *pData) {
1298+
HashTable *ht = *((HashTable **)pData);
1299+
1300+
#if PHP_VERSION_ID < 70400
1301+
zend_array_release(ht);
1302+
#else
1303+
zend_release_properties(ht);
1304+
#endif
1305+
}
1306+
12971307
void ddtrace_live_debugger_minit(void) {
1308+
zend_hash_init(&DDTRACE_G(debugger_capture_ephemerals), 8, NULL, (dtor_func_t)dd_ht_ephemerals_dtor, 1);
1309+
12981310
zend_string *value;
12991311
ZEND_HASH_FOREACH_STR_KEY(get_global_DD_DYNAMIC_INSTRUMENTATION_REDACTED_IDENTIFIERS(), value) {
13001312
ddog_snapshot_add_redacted_name(dd_zend_string_to_CharSlice(value));
@@ -1303,3 +1315,7 @@ void ddtrace_live_debugger_minit(void) {
13031315
ddog_snapshot_add_redacted_type(dd_zend_string_to_CharSlice(value));
13041316
} ZEND_HASH_FOREACH_END();
13051317
}
1318+
1319+
void ddtrace_live_debugger_mshutdown(void) {
1320+
zend_hash_destroy(&DDTRACE_G(debugger_capture_ephemerals));
1321+
}

‎ext/live_debugger.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
extern ddog_LiveDebuggerSetup ddtrace_live_debugger_setup;
77

88
void ddtrace_live_debugger_minit(void);
9+
void ddtrace_live_debugger_mshutdown(void);
910

1011
static inline void ddtrace_snapshot_redacted_name(ddog_CaptureValue *capture_value, ddog_CharSlice name) {
1112
if (ddog_snapshot_redacted_name(name)) {

‎ext/sidecar.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,11 +378,13 @@ void ddtrace_sidecar_submit_root_span_data(void) {
378378
void ddtrace_sidecar_send_debugger_data(ddog_Vec_DebuggerPayload payloads) {
379379
LOGEV(DEBUG, UNUSED(log); ddog_log_debugger_data(&payloads););
380380
ddog_sidecar_send_debugger_data(&ddtrace_sidecar, ddtrace_sidecar_instance_id, DDTRACE_G(sidecar_queue_id), payloads);
381+
zend_hash_clean(&DDTRACE_G(debugger_capture_ephemerals));
381382
}
382383

383384
void ddtrace_sidecar_send_debugger_datum(ddog_DebuggerPayload *payload) {
384385
LOGEV(DEBUG, UNUSED(log); ddog_log_debugger_datum(payload););
385386
ddog_sidecar_send_debugger_datum(&ddtrace_sidecar, ddtrace_sidecar_instance_id, DDTRACE_G(sidecar_queue_id), payload);
387+
zend_hash_clean(&DDTRACE_G(debugger_capture_ephemerals));
386388
}
387389

388390
void ddtrace_sidecar_activate(void) {

‎tests/ext/live-debugger/exception-replay_non_regression_2989_mysqli.phpt

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
--TEST--
2+
Non regression test for use-after-free segfault in exception replay
3+
--SKIPIF--
4+
<?php if (!extension_loaded('mysqli')) die('skip: mysqli extension required'); ?>
5+
<?php include __DIR__ . '/../includes/skipif_no_dev_env.inc'; ?>
6+
<?php if (getenv('PHP_PEAR_RUNTESTS') === '1') die("skip: pecl run-tests does not support %A in EXPECTF"); ?>
7+
--ENV--
8+
DD_AGENT_HOST=request-replayer
9+
DD_TRACE_AGENT_PORT=80
10+
DD_TRACE_GENERATE_ROOT_SPAN=0
11+
DD_EXCEPTION_REPLAY_ENABLED=1
12+
DD_EXCEPTION_REPLAY_CAPTURE_INTERVAL_SECONDS=1
13+
--INI--
14+
datadog.trace.agent_test_session_token=live-debugger/non_regression_2989_mysqli
15+
--FILE--
16+
<?php
17+
18+
require __DIR__ . "/live_debugger.inc";
19+
20+
mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);
21+
22+
try {
23+
$mysqli = new mysqli("@@_INVALID_@@", "foo", "bar");
24+
} catch (Exception $e) {
25+
$span = \DDTrace\start_span();
26+
$span->exception = $e;
27+
\DDTrace\close_span();
28+
}
29+
30+
$dlr = new DebuggerLogReplayer;
31+
$log = $dlr->waitForDebuggerDataAndReplay();
32+
$log = json_decode($log["body"], true);
33+
34+
function recursive_ksort(&$arr) {
35+
if (is_array($arr)) {
36+
ksort($arr);
37+
array_walk($arr, 'recursive_ksort');
38+
}
39+
}
40+
41+
recursive_ksort($log[0]["debugger"]["snapshot"]["captures"]);
42+
var_dump($log[0]);
43+
44+
?>
45+
--CLEAN--
46+
<?php
47+
require __DIR__ . "/live_debugger.inc";
48+
reset_request_replayer();
49+
?>
50+
--EXPECTF--
51+
Warning: mysqli::__construct(): php_network_getaddresses: getaddrinfo %s
52+
%Aarray(5) {
53+
["service"]=>
54+
string(47) "exception-replay_non_regression_2989_mysqli.php"
55+
["ddsource"]=>
56+
string(11) "dd_debugger"
57+
["timestamp"]=>
58+
int(%d)
59+
["debugger"]=>
60+
array(1) {
61+
["snapshot"]=>
62+
array(8) {
63+
["language"]=>
64+
string(3) "php"
65+
["id"]=>
66+
string(36) "%s"
67+
["timestamp"]=>
68+
int(%d)
69+
["exceptionCaptureId"]=>
70+
string(36) "%s"
71+
["exceptionHash"]=>
72+
string(16) "%s"
73+
["frameIndex"]=>
74+
int(0)
75+
["captures"]=>
76+
array(1) {
77+
["return"]=>
78+
array(1) {
79+
["arguments"]=>
80+
array(4) {
81+
["hos%s"]=>
82+
array(2) {
83+
["type"]=>
84+
string(6) "string"
85+
["value"]=>
86+
string(13) "@@_INVALID_@@"
87+
}
88+
["password"]=>
89+
array(%d) {
90+
%A
91+
}
92+
["this"]=>
93+
array(2) {
94+
["fields"]=>
95+
array(%d) {
96+
%A
97+
}
98+
["type"]=>
99+
string(6) "mysqli"
100+
}
101+
["use%s"]=>
102+
array(2) {
103+
["type"]=>
104+
string(6) "string"
105+
["value"]=>
106+
string(3) "foo"
107+
}
108+
}
109+
}
110+
}
111+
["probe"]=>
112+
array(2) {
113+
["id"]=>
114+
string(0) ""
115+
["location"]=>
116+
array(2) {
117+
["method"]=>
118+
string(11) "__construct"
119+
["type"]=>
120+
string(6) "mysqli"
121+
}
122+
}
123+
}
124+
}
125+
["message"]=>
126+
NULL
127+
}

0 commit comments

Comments
 (0)