Skip to content

Commit

Permalink
Fix use-after-free with live-debugger (#2989)
Browse files Browse the repository at this point in the history
* Fix user-after-free with debugger

* Avoid copies

* fix compilation + fix on PHP < 7.4
  • Loading branch information
iamluc authored Jan 7, 2025
1 parent 4a6498d commit cc11b8e
Show file tree
Hide file tree
Showing 7 changed files with 157 additions and 4 deletions.
2 changes: 2 additions & 0 deletions ext/ddtrace.c
Original file line number Diff line number Diff line change
Expand Up @@ -1508,6 +1508,8 @@ static PHP_MSHUTDOWN_FUNCTION(ddtrace) {

ddtrace_sidecar_shutdown();

ddtrace_live_debugger_mshutdown();

#if PHP_VERSION_ID >= 80000 && PHP_VERSION_ID < 80100
// See dd_register_span_data_ce for explanation
zend_string *key;
Expand Down
1 change: 1 addition & 0 deletions ext/ddtrace.h
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ ZEND_BEGIN_MODULE_GLOBALS(ddtrace)
ddog_RemoteConfigState *remote_config_state;
ddog_AgentInfoReader *agent_info_reader;
zend_arena *debugger_capture_arena;
HashTable debugger_capture_ephemerals;
ddog_Vec_DebuggerPayload exception_debugger_buffer;
HashTable active_rc_hooks;
HashTable *agent_rate_by_service;
Expand Down
10 changes: 7 additions & 3 deletions ext/exception_serialize.c
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,10 @@ void ddtrace_create_capture_value(zval *zv, struct ddog_CaptureValue *value, con
zend_get_properties_for(zv, ZEND_PROP_PURPOSE_DEBUG)
#endif
: Z_OBJPROP_P(zv);

if (!ht) {
break;
}
ZEND_HASH_REVERSE_FOREACH_STR_KEY_VAL(ht, key, val) {
if (!key) {
continue;
Expand Down Expand Up @@ -263,10 +267,10 @@ void ddtrace_create_capture_value(zval *zv, struct ddog_CaptureValue *value, con
if (ce->type == ZEND_INTERNAL_CLASS) {
#if PHP_VERSION_ID < 70400
if (is_temp) {
zend_array_release(ht);
zend_hash_next_index_insert_ptr(&DDTRACE_G(debugger_capture_ephemerals), ht);
}
#else
zend_release_properties(ht);
zend_hash_next_index_insert_ptr(&DDTRACE_G(debugger_capture_ephemerals), ht);
#endif
}
break;
Expand Down Expand Up @@ -392,7 +396,7 @@ static void ddtrace_collect_exception_debug_data(zend_object *exception, zend_st
LOG(TRACE, "Skipping exception replay capture due to hash %.*s already recently hit", hash_len, exception_hash);
return;
}

char *exception_id = zend_arena_alloc(&DDTRACE_G(debugger_capture_arena), uuid_len);
ddog_snapshot_format_new_uuid((uint8_t(*)[uuid_len])exception_id);

Expand Down
18 changes: 17 additions & 1 deletion ext/live_debugger.c
Original file line number Diff line number Diff line change
Expand Up @@ -781,7 +781,7 @@ static const void *dd_eval_fetch_identifier(void *ctx, const ddog_CharSlice *nam
}
return NULL;
}

return NULL;
}

Expand Down Expand Up @@ -1294,7 +1294,19 @@ ddog_LiveDebuggerSetup ddtrace_live_debugger_setup = {
.evaluator = &dd_evaluator,
};

static void dd_ht_ephemerals_dtor(void *pData) {
HashTable *ht = *((HashTable **)pData);

#if PHP_VERSION_ID < 70400
zend_array_release(ht);
#else
zend_release_properties(ht);
#endif
}

void ddtrace_live_debugger_minit(void) {
zend_hash_init(&DDTRACE_G(debugger_capture_ephemerals), 8, NULL, (dtor_func_t)dd_ht_ephemerals_dtor, 1);

zend_string *value;
ZEND_HASH_FOREACH_STR_KEY(get_global_DD_DYNAMIC_INSTRUMENTATION_REDACTED_IDENTIFIERS(), value) {
ddog_snapshot_add_redacted_name(dd_zend_string_to_CharSlice(value));
Expand All @@ -1303,3 +1315,7 @@ void ddtrace_live_debugger_minit(void) {
ddog_snapshot_add_redacted_type(dd_zend_string_to_CharSlice(value));
} ZEND_HASH_FOREACH_END();
}

void ddtrace_live_debugger_mshutdown(void) {
zend_hash_destroy(&DDTRACE_G(debugger_capture_ephemerals));
}
1 change: 1 addition & 0 deletions ext/live_debugger.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
extern ddog_LiveDebuggerSetup ddtrace_live_debugger_setup;

void ddtrace_live_debugger_minit(void);
void ddtrace_live_debugger_mshutdown(void);

static inline void ddtrace_snapshot_redacted_name(ddog_CaptureValue *capture_value, ddog_CharSlice name) {
if (ddog_snapshot_redacted_name(name)) {
Expand Down
2 changes: 2 additions & 0 deletions ext/sidecar.c
Original file line number Diff line number Diff line change
Expand Up @@ -378,11 +378,13 @@ void ddtrace_sidecar_submit_root_span_data(void) {
void ddtrace_sidecar_send_debugger_data(ddog_Vec_DebuggerPayload payloads) {
LOGEV(DEBUG, UNUSED(log); ddog_log_debugger_data(&payloads););
ddog_sidecar_send_debugger_data(&ddtrace_sidecar, ddtrace_sidecar_instance_id, DDTRACE_G(sidecar_queue_id), payloads);
zend_hash_clean(&DDTRACE_G(debugger_capture_ephemerals));
}

void ddtrace_sidecar_send_debugger_datum(ddog_DebuggerPayload *payload) {
LOGEV(DEBUG, UNUSED(log); ddog_log_debugger_datum(payload););
ddog_sidecar_send_debugger_datum(&ddtrace_sidecar, ddtrace_sidecar_instance_id, DDTRACE_G(sidecar_queue_id), payload);
zend_hash_clean(&DDTRACE_G(debugger_capture_ephemerals));
}

void ddtrace_sidecar_activate(void) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
--TEST--
Non regression test for use-after-free segfault in exception replay
--SKIPIF--
<?php if (!extension_loaded('mysqli')) die('skip: mysqli extension required'); ?>
<?php include __DIR__ . '/../includes/skipif_no_dev_env.inc'; ?>
<?php if (getenv('PHP_PEAR_RUNTESTS') === '1') die("skip: pecl run-tests does not support %A in EXPECTF"); ?>
--ENV--
DD_AGENT_HOST=request-replayer
DD_TRACE_AGENT_PORT=80
DD_TRACE_GENERATE_ROOT_SPAN=0
DD_EXCEPTION_REPLAY_ENABLED=1
DD_EXCEPTION_REPLAY_CAPTURE_INTERVAL_SECONDS=1
--INI--
datadog.trace.agent_test_session_token=live-debugger/non_regression_2989_mysqli
--FILE--
<?php

require __DIR__ . "/live_debugger.inc";

mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);

try {
$mysqli = new mysqli("@@_INVALID_@@", "foo", "bar");
} catch (Exception $e) {
$span = \DDTrace\start_span();
$span->exception = $e;
\DDTrace\close_span();
}

$dlr = new DebuggerLogReplayer;
$log = $dlr->waitForDebuggerDataAndReplay();
$log = json_decode($log["body"], true);

function recursive_ksort(&$arr) {
if (is_array($arr)) {
ksort($arr);
array_walk($arr, 'recursive_ksort');
}
}

recursive_ksort($log[0]["debugger"]["snapshot"]["captures"]);
var_dump($log[0]);

?>
--CLEAN--
<?php
require __DIR__ . "/live_debugger.inc";
reset_request_replayer();
?>
--EXPECTF--
Warning: mysqli::__construct(): php_network_getaddresses: getaddrinfo %s
%Aarray(5) {
["service"]=>
string(47) "exception-replay_non_regression_2989_mysqli.php"
["ddsource"]=>
string(11) "dd_debugger"
["timestamp"]=>
int(%d)
["debugger"]=>
array(1) {
["snapshot"]=>
array(8) {
["language"]=>
string(3) "php"
["id"]=>
string(36) "%s"
["timestamp"]=>
int(%d)
["exceptionCaptureId"]=>
string(36) "%s"
["exceptionHash"]=>
string(16) "%s"
["frameIndex"]=>
int(0)
["captures"]=>
array(1) {
["return"]=>
array(1) {
["arguments"]=>
array(4) {
["hos%s"]=>
array(2) {
["type"]=>
string(6) "string"
["value"]=>
string(13) "@@_INVALID_@@"
}
["password"]=>
array(%d) {
%A
}
["this"]=>
array(2) {
["fields"]=>
array(%d) {
%A
}
["type"]=>
string(6) "mysqli"
}
["use%s"]=>
array(2) {
["type"]=>
string(6) "string"
["value"]=>
string(3) "foo"
}
}
}
}
["probe"]=>
array(2) {
["id"]=>
string(0) ""
["location"]=>
array(2) {
["method"]=>
string(11) "__construct"
["type"]=>
string(6) "mysqli"
}
}
}
}
["message"]=>
NULL
}

0 comments on commit cc11b8e

Please sign in to comment.