From cc11b8e4baa960e58265ae7b6cc72a39968ddddb Mon Sep 17 00:00:00 2001 From: Luc Vieillescazes Date: Tue, 7 Jan 2025 14:20:48 +0100 Subject: [PATCH] Fix use-after-free with live-debugger (#2989) * Fix user-after-free with debugger * Avoid copies * fix compilation + fix on PHP < 7.4 --- ext/ddtrace.c | 2 + ext/ddtrace.h | 1 + ext/exception_serialize.c | 10 +- ext/live_debugger.c | 18 ++- ext/live_debugger.h | 1 + ext/sidecar.c | 2 + ...ion-replay_non_regression_2989_mysqli.phpt | 127 ++++++++++++++++++ 7 files changed, 157 insertions(+), 4 deletions(-) create mode 100644 tests/ext/live-debugger/exception-replay_non_regression_2989_mysqli.phpt diff --git a/ext/ddtrace.c b/ext/ddtrace.c index f4b6d32a5e..0e24b030a4 100644 --- a/ext/ddtrace.c +++ b/ext/ddtrace.c @@ -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; diff --git a/ext/ddtrace.h b/ext/ddtrace.h index 19fde78a08..899e90332b 100644 --- a/ext/ddtrace.h +++ b/ext/ddtrace.h @@ -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; diff --git a/ext/exception_serialize.c b/ext/exception_serialize.c index 909f506fd4..874450d679 100644 --- a/ext/exception_serialize.c +++ b/ext/exception_serialize.c @@ -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; @@ -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; @@ -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); diff --git a/ext/live_debugger.c b/ext/live_debugger.c index 2b1b9335e1..194640cfe4 100644 --- a/ext/live_debugger.c +++ b/ext/live_debugger.c @@ -781,7 +781,7 @@ static const void *dd_eval_fetch_identifier(void *ctx, const ddog_CharSlice *nam } return NULL; } - + return NULL; } @@ -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)); @@ -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)); +} diff --git a/ext/live_debugger.h b/ext/live_debugger.h index 8759466b2c..8af25cc719 100644 --- a/ext/live_debugger.h +++ b/ext/live_debugger.h @@ -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)) { diff --git a/ext/sidecar.c b/ext/sidecar.c index e616565481..c43c5928ba 100644 --- a/ext/sidecar.c +++ b/ext/sidecar.c @@ -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) { diff --git a/tests/ext/live-debugger/exception-replay_non_regression_2989_mysqli.phpt b/tests/ext/live-debugger/exception-replay_non_regression_2989_mysqli.phpt new file mode 100644 index 0000000000..a684465fab --- /dev/null +++ b/tests/ext/live-debugger/exception-replay_non_regression_2989_mysqli.phpt @@ -0,0 +1,127 @@ +--TEST-- +Non regression test for use-after-free segfault in exception replay +--SKIPIF-- + + + +--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-- +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-- + +--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 +}