fix: sentry-trace header parsing

CHANGELOG.md

@@ -23,6 +23,7 @@
 - Cap rate-limit retry-after values at 24 hours to prevent a MITM-provided response from disabling event delivery for the process lifetime. ([#1744](https://github.com/getsentry/sentry-native/pull/1744))
 - Native: validate ELF header entry sizes. ([#1746](https://github.com/getsentry/sentry-native/pull/1746))
 - Structured logs: respect printf argument widths when extracting log parameters to avoid stack-data disclosure and corrupted attributes on 32-bit platforms. ([#1752](https://github.com/getsentry/sentry-native/pull/1752))
+- Fix a potential out-of-bounds read when parsing non-NUL-terminated `sentry-trace` headers. ([#1749](https://github.com/getsentry/sentry-native/pull/1749))
 
 ## 0.14.2
 

src/sentry_tracing.c

@@ -324,10 +324,15 @@ parse_sentry_trace(
     }
 
     const char *span_id_start = trace_id_end + 1;
-    const char *span_id_end = strchr(span_id_start, '-');
+    const char *value_end = value + value_len;
+    const char *span_id_end
+        = memchr(span_id_start, '-', (size_t)(value_end - span_id_start));
+    const size_t span_id_len
+        = (size_t)((span_id_end ? span_id_end : value_end) - span_id_start);
     if (!span_id_end) {
         // no sampled flag
-        sentry_value_t parent_span_id = sentry_value_new_string(span_id_start);
+        sentry_value_t parent_span_id
+            = sentry_value_new_string_n(span_id_start, span_id_len);
         if (!is_valid_span_id(sentry_value_as_string(parent_span_id))) {
             sentry_value_decref(parent_span_id);
             return;
@@ -337,16 +342,15 @@ parse_sentry_trace(
     }
     // else: we have a sampled flag
 
-    s = sentry__string_clone_n(
-        span_id_start, (size_t)(span_id_end - span_id_start));
+    s = sentry__string_clone_n(span_id_start, span_id_len);
     if (!is_valid_span_id(s)) {
         sentry_free(s);
         return;
     }
     sentry_value_t parent_span_id = sentry__value_new_string_owned(s);
     sentry_value_set_by_key(inner, "parent_span_id", parent_span_id);
 
-    bool sampled = *(span_id_end + 1) == '1';
+    bool sampled = span_id_end + 1 < value_end && *(span_id_end + 1) == '1';
     sentry_value_set_by_key(inner, "sampled", sentry_value_new_bool(sampled));
 }
 

tests/unit/test_tracing.c

@@ -1190,6 +1190,31 @@ SENTRY_TEST(distributed_headers)
     sentry_close();
 }
 
+SENTRY_TEST(distributed_headers_invalid_len)
+{
+    SENTRY_TEST_OPTIONS_NEW(options);
+    sentry_options_set_dsn(options, "https://foo@sentry.invalid/42");
+
+    sentry_init(options);
+
+    sentry_transaction_context_t *tx_ctx
+        = sentry_transaction_context_new("wow!", NULL);
+    const char trace_header[]
+        = "2674eb52d5874b13b560236d6c79ce8a-a0f9fdf04f1a63df-1";
+    const size_t trace_header_len = 32 + 1 + 8;
+
+    sentry_transaction_context_update_from_header_n(tx_ctx, "sentry-trace",
+        strlen("sentry-trace"), trace_header, trace_header_len);
+
+    TEST_CHECK(sentry_value_is_null(
+        sentry_value_get_by_key(tx_ctx->inner, "parent_span_id")));
+    TEST_CHECK(sentry_value_is_null(
+        sentry_value_get_by_key(tx_ctx->inner, "sampled")));
+
+    sentry__transaction_context_free(tx_ctx);
+    sentry_close();
+}
+
 void
 check_after_set(sentry_value_t inner, const char *inner_key,
     const char *item_key, const char *expected)

tests/unit/tests.inc

@@ -105,6 +105,7 @@ XX(deserialize_envelope_invalid)
 XX(deserialize_envelope_no_headers)
 XX(discarding_before_send)
 XX(distributed_headers)
+XX(distributed_headers_invalid_len)
 XX(distributed_headers_invalid_spanid)
 XX(distributed_headers_invalid_traceid)
 XX(drop_unfinished_spans)