From b9efbb82a2e3a37eeea207d8b7045aa87518a45f Mon Sep 17 00:00:00 2001 From: Bob Weinand Date: Mon, 20 Jan 2025 16:04:14 +0100 Subject: [PATCH] Validate DD_TRACE_AGENT_URL This avoids panics in the sidecar and crashes in crashtracker initialization. Signed-off-by: Bob Weinand --- Cargo.lock | 1 + components-rs/Cargo.toml | 1 + components-rs/ddtrace.h | 2 ++ components-rs/lib.rs | 18 ++++++++++++++++++ ext/sidecar.c | 10 +++++++--- ext/signals.c | 3 +++ .../sidecar_handles_invalid_agent_url.phpt | 15 +++++++++++++++ 7 files changed, 47 insertions(+), 3 deletions(-) create mode 100644 tests/ext/background-sender/sidecar_handles_invalid_agent_url.phpt diff --git a/Cargo.lock b/Cargo.lock index 354fbed663..72bf7a5f96 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1860,6 +1860,7 @@ dependencies = [ "ddtelemetry", "ddtelemetry-ffi", "env_logger 0.10.2", + "http 0.2.11", "itertools 0.11.0", "lazy_static", "log", diff --git a/components-rs/Cargo.toml b/components-rs/Cargo.toml index 3f7cf28b46..dc4d84050a 100644 --- a/components-rs/Cargo.toml +++ b/components-rs/Cargo.toml @@ -43,6 +43,7 @@ tracing-subscriber = { version = "0.3", default-features = false, features = [ serde_json = "1.0.113" regex = "1.10.5" regex-automata = "0.4.5" +http = "0.2.11" [build-dependencies] cbindgen = "0.27" diff --git a/components-rs/ddtrace.h b/components-rs/ddtrace.h index 2a6b4f24ad..fbc10a11ab 100644 --- a/components-rs/ddtrace.h +++ b/components-rs/ddtrace.h @@ -153,6 +153,8 @@ char *ddtrace_strip_invalid_utf8(const char *input, uintptr_t *len); void ddtrace_drop_rust_string(char *input, uintptr_t len); +struct ddog_Endpoint *ddtrace_parse_agent_url(ddog_CharSlice url); + bool ddog_shall_log(enum ddog_Log category); void ddog_set_error_log_level(bool once); diff --git a/components-rs/lib.rs b/components-rs/lib.rs index 49db14b504..2a4417d417 100644 --- a/components-rs/lib.rs +++ b/components-rs/lib.rs @@ -10,11 +10,14 @@ pub mod telemetry; use std::borrow::Cow; use std::ffi::c_char; use std::ptr::null_mut; +use http::Uri; use ddcommon::entity_id::{get_container_id, set_cgroup_file}; +use http::uri::Scheme; use uuid::Uuid; pub use datadog_crashtracker_ffi::*; pub use datadog_sidecar_ffi::*; +use ddcommon::{parse_uri, Endpoint}; use ddcommon_ffi::slice::AsBytes; pub use ddcommon_ffi::*; pub use ddtelemetry_ffi::*; @@ -65,3 +68,18 @@ pub unsafe extern "C" fn ddtrace_strip_invalid_utf8(input: *const c_char, len: * pub unsafe extern "C" fn ddtrace_drop_rust_string(input: *mut c_char, len: usize) { _ = String::from_raw_parts(input as *mut u8, len, len); } + +#[no_mangle] +pub unsafe extern "C" fn ddtrace_parse_agent_url(url: CharSlice) -> std::option::Option> { + parse_uri(url.to_utf8_lossy().as_ref()) + .ok() + .and_then(|url| if url.authority().is_none() { None } else { Some(url) }) + .map(|mut url| { + if url.scheme().is_none() { + let mut parts = url.into_parts(); + parts.scheme = Some(Scheme::HTTP); + url = Uri::from_parts(parts).unwrap(); + } + Box::new(Endpoint::from_url(url)) + }) +} diff --git a/ext/sidecar.c b/ext/sidecar.c index c43c5928ba..7ec14b28e9 100644 --- a/ext/sidecar.c +++ b/ext/sidecar.c @@ -37,7 +37,7 @@ static void ddtrace_set_resettable_sidecar_globals(void) { } ddog_SidecarTransport *dd_sidecar_connection_factory(void) { - // Should not happen + // Should not happen, unless the agent url is malformed if (!ddtrace_endpoint) { return NULL; } @@ -171,11 +171,15 @@ ddog_Endpoint *ddtrace_sidecar_agent_endpoint(void) { agent_endpoint = ddog_endpoint_from_api_key(dd_zend_string_to_CharSlice(get_global_DD_API_KEY())); } else { char *agent_url = ddtrace_agent_url(); - agent_endpoint = ddog_endpoint_from_url((ddog_CharSlice) {.ptr = agent_url, .len = strlen(agent_url)}); + agent_endpoint = ddtrace_parse_agent_url((ddog_CharSlice) {.ptr = agent_url, .len = strlen(agent_url)}); + if (!agent_endpoint) { + LOG(ERROR, "Invalid DD_TRACE_AGENT_URL: %s. A proper agent URL must be unix:///path/to/agent.sock or http://hostname:port/.", agent_url); + } free(agent_url); } - if (ZSTR_LEN(get_global_DD_TRACE_AGENT_TEST_SESSION_TOKEN())) { + + if (agent_endpoint && ZSTR_LEN(get_global_DD_TRACE_AGENT_TEST_SESSION_TOKEN())) { ddog_endpoint_set_test_token(agent_endpoint, dd_zend_string_to_CharSlice(get_global_DD_TRACE_AGENT_TEST_SESSION_TOKEN())); } diff --git a/ext/signals.c b/ext/signals.c index ce51361b9e..dd47811d89 100644 --- a/ext/signals.c +++ b/ext/signals.c @@ -117,6 +117,9 @@ static void ddtrace_init_crashtracker() { socket_path.ptr = crashtracker_socket_path; ddog_Endpoint *agent_endpoint = ddtrace_sidecar_agent_endpoint(); + if (!agent_endpoint) { + return; + } ddog_crasht_Config config = { .endpoint = agent_endpoint, diff --git a/tests/ext/background-sender/sidecar_handles_invalid_agent_url.phpt b/tests/ext/background-sender/sidecar_handles_invalid_agent_url.phpt new file mode 100644 index 0000000000..4665a9a110 --- /dev/null +++ b/tests/ext/background-sender/sidecar_handles_invalid_agent_url.phpt @@ -0,0 +1,15 @@ +--TEST-- +The sidecar properly handles invalid agent urls +--SKIPIF-- + + +--ENV-- +DD_TRACE_AGENT_URL=/invalid +DD_TRACE_SIDECAR_TRACE_SENDER=1 +DD_CRASHTRACKING_ENABLED=0 +--FILE-- + +--EXPECTF-- +[ddtrace] [error] Invalid DD_TRACE_AGENT_URL: /invalid. A proper agent URL must be unix:///path/to/agent.sock or http://hostname:port/.