Validate DD_TRACE_AGENT_URL

This avoids panics in the sidecar and crashes in crashtracker initialization. Signed-off-by: Bob Weinand <[email protected]>
DataDog · Jan 20, 2025 · b9efbb8 · b9efbb8
1 parent 7a0d358
commit b9efbb8
Show file tree

Hide file tree

Showing 7 changed files with 47 additions and 3 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git 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
@@ -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
@@ -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<Box<Endpoint>> {
+    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
@@ -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
@@ -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
@@ -0,0 +1,15 @@
+--TEST--
+The sidecar properly handles invalid agent urls
+--SKIPIF--
+<?php include __DIR__ . '/../includes/skipif_no_dev_env.inc'; ?>
+<?php if (getenv('USE_ZEND_ALLOC') === '0' && !getenv("SKIP_ASAN")) die('skip: valgrind reports sendmsg(msg.msg_control) points to uninitialised byte(s), but it is unproblematic and outside our control in rust code'); ?>
+--ENV--
+DD_TRACE_AGENT_URL=/invalid
+DD_TRACE_SIDECAR_TRACE_SENDER=1
+DD_CRASHTRACKING_ENABLED=0
+--FILE--
+<?php
+
+?>
+--EXPECTF--
+[ddtrace] [error] Invalid DD_TRACE_AGENT_URL: /invalid. A proper agent URL must be unix:///path/to/agent.sock or http://hostname:port/.