Crashtracker: fix a use-after-free error (#2843)

* Crashtracker: fix a use-after-free error * Set DD_LOG_BACKTRACE default value to "false" when compiled with libasan
DataDog · Sep 10, 2024 · 459fe08 · 459fe08
1 parent 98839cf
commit 459fe08
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 7 deletions.
diff --git a/ext/configuration.h b/ext/configuration.h
@@ -90,6 +90,12 @@ enum ddtrace_sampling_rules_format {
 #define DD_TRACE_OBFUSCATION_QUERY_STRING_REGEXP_DEFAULT \
     "(?i)(?:(?:\"|%22)?)(?:(?:old[-_]?|new[-_]?)?p(?:ass)?w(?:or)?d(?:1|2)?|pass(?:[-_]?phrase)?|secret|(?:api[-_]?|private[-_]?|public[-_]?|access[-_]?|secret[-_]?|app(?:lication)?[-_]?)key(?:[-_]?id)?|token|consumer[-_]?(?:id|key|secret)|sign(?:ed|ature)?|auth(?:entication|orization)?)(?:(?:\\s|%20)*(?:=|%3D)[^&]+|(?:\"|%22)(?:\\s|%20)*(?::|%3A)(?:\\s|%20)*(?:\"|%22)(?:%2[^2]|%[^2]|[^\"%])+(?:\"|%22))|(?:bearer(?:\\s|%20)+[a-z0-9._\\-]+|token(?::|%3A)[a-z0-9]{13}|gh[opsu]_[0-9a-zA-Z]{36}|ey[I-L](?:[\\w=-]|%3D)+\\.ey[I-L](?:[\\w=-]|%3D)+(?:\\.(?:[\\w.+/=-]|%3D|%2F|%2B)+)?|-{5}BEGIN(?:[a-z\\s]|%20)+PRIVATE(?:\\s|%20)KEY-{5}[^\\-]+-{5}END(?:[a-z\\s]|%20)+PRIVATE(?:\\s|%20)KEY(?:-{5})?(?:\\n|%0A)?|(?:ssh-(?:rsa|dss)|ecdsa-[a-z0-9]+-[a-z0-9]+)(?:\\s|%20|%09)+(?:[a-z0-9/.+]|%2F|%5C|%2B){100,}(?:=|%3D)*(?:(?:\\s|%20|%09)+[a-z0-9._-]+)?)"
 
+#ifdef __SANITIZE_ADDRESS__
+#define DD_LOG_BACKTRACE_DEFAULT "false"
+#else
+#define DD_LOG_BACKTRACE_DEFAULT "true"
+#endif
+
 #define DD_CONFIGURATION_ALL                                                                                   \
     CONFIG(STRING, DD_TRACE_SOURCES_PATH, DD_DEFAULT_SOURCES_PATH, .ini_change = zai_config_system_ini_change) \
     CONFIG(BOOL, DD_AUTOLOAD_NO_COMPILE, "false", .ini_change = zai_config_system_ini_change)                  \
@@ -163,7 +169,7 @@ enum ddtrace_sampling_rules_format {
     CONFIG(INT, DD_TRACE_AGENT_CONNECT_TIMEOUT, DD_CFG_EXPSTR(DD_TRACE_AGENT_CONNECT_TIMEOUT_VAL),             \
            .ini_change = zai_config_system_ini_change)                                                         \
     CONFIG(INT, DD_TRACE_DEBUG_PRNG_SEED, "-1", .ini_change = ddtrace_reseed_seed_change)                      \
-    CONFIG(BOOL, DD_LOG_BACKTRACE, "true")                                                                    \
+    CONFIG(BOOL, DD_LOG_BACKTRACE, DD_LOG_BACKTRACE_DEFAULT)                                                                    \
     CONFIG(BOOL, DD_TRACE_GENERATE_ROOT_SPAN, "true", .ini_change = ddtrace_span_alter_root_span_config)       \
     CONFIG(INT, DD_TRACE_SPANS_LIMIT, "1000")                                                                  \
     CONFIG(BOOL, DD_TRACE_128_BIT_TRACEID_GENERATION_ENABLED, "true")                                          \

diff --git a/ext/signals.c b/ext/signals.c
@@ -37,7 +37,7 @@
 // true globals; only modify in MINIT/MSHUTDOWN
 static stack_t ddtrace_altstack;
 static struct sigaction ddtrace_sigaction;
-static ddog_CharSlice crashtracker_socket_path = {0};
+static char crashtracker_socket_path[100] = {0};
 
 #define MAX_STACK_SIZE 1024
 #define MIN_STACKSZ 16384  // enough to hold void *array[MAX_STACK_SIZE] plus a couple kilobytes
@@ -101,8 +101,20 @@ static bool ddtrace_crashtracker_check_result(ddog_crasht_Result result, const c
 }
 
 static void ddtrace_init_crashtracker() {
+    ddog_CharSlice socket_path = ddog_sidecar_get_crashtracker_unix_socket_path();
+    if (socket_path.len > sizeof(crashtracker_socket_path) - 1) {
+        LOG(ERROR, "Cannot initialize CrashTracker : the socket path is too long.");
+        free((void *) socket_path.ptr);
+        return;
+    }
+
+    // Copy the string to a global buffer to avoid a use-after-free error
+    memcpy(crashtracker_socket_path, socket_path.ptr, socket_path.len);
+    crashtracker_socket_path[socket_path.len] = '\0';
+    free((void *) socket_path.ptr);
+    socket_path.ptr = crashtracker_socket_path;
+
     ddog_Endpoint *agent_endpoint = ddtrace_sidecar_agent_endpoint();
-    crashtracker_socket_path = ddog_sidecar_get_crashtracker_unix_socket_path();
 
     ddog_crasht_Config config = {
         .endpoint = agent_endpoint,
@@ -139,7 +151,7 @@ static void ddtrace_init_crashtracker() {
     ddtrace_crashtracker_check_result(
         ddog_crasht_init_with_unix_socket(
             config,
-            crashtracker_socket_path,
+            socket_path,
             metadata
         ),
         "Cannot initialize CrashTracker"
@@ -183,9 +195,6 @@ void ddtrace_signals_first_rinit(void) {
 
 void ddtrace_signals_mshutdown(void) {
     free(ddtrace_altstack.ss_sp);
-    if (crashtracker_socket_path.ptr) {
-        free((void *) crashtracker_socket_path.ptr);
-    }
 }
 
 #else