From 38c6c14aad025bee4e1d2a56bc81919b852194ef Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Thu, 15 Feb 2024 11:34:10 +0100 Subject: [PATCH 1/2] fix(spans): Max description length --- .../src/normalize/span/description/mod.rs | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/relay-event-normalization/src/normalize/span/description/mod.rs b/relay-event-normalization/src/normalize/span/description/mod.rs index 7e5e3ca8f8..232262c85f 100644 --- a/relay-event-normalization/src/normalize/span/description/mod.rs +++ b/relay-event-normalization/src/normalize/span/description/mod.rs @@ -20,6 +20,9 @@ use crate::span::tag_extraction::HTTP_METHOD_EXTRACTOR_REGEX; /// Dummy URL used to parse relative URLs. static DUMMY_BASE_URL: Lazy = Lazy::new(|| "http://replace_me".parse().unwrap()); +/// Very large SQL queries may cause stack overflows in the parser, so do not attempt to parse these. +const MAX_DESCRIPTION_LENGTH: usize = 10_000; + /// Maximum length of a resource URL segment. /// /// Segments longer than this are treated as identifiers. @@ -38,6 +41,14 @@ pub(crate) fn scrub_span_description( return (None, None); }; + if description.len() > MAX_DESCRIPTION_LENGTH { + relay_log::error!( + description = description, + "Span description too large to parse" + ); + return (None, None); + } + let data = span.data.value(); let db_system = data @@ -421,7 +432,7 @@ mod tests { // Same output and input means the input was already scrubbed. // An empty output `""` means the input wasn't scrubbed and Relay didn't scrub it. - ($name:ident, $description_in:literal, $op_in:literal, $expected:literal) => { + ($name:ident, $description_in:expr, $op_in:literal, $expected:literal) => { #[test] fn $name() { let json = format!( @@ -911,6 +922,17 @@ mod tests { span_description_test!(db_prisma, "User find", "db.sql.prisma", "User find"); + span_description_test!( + long_description_none, + // Do not attempt to parse very long descriptions. + { + let repeated = "+1".repeat(5000); + &("SELECT 1".to_string() + &repeated) + }, + "db.query", + "" + ); + #[test] fn informed_sql_parser() { let json = r#" From c0e870c69a1eda3ca91918469b90d4dc81c4f388 Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Thu, 15 Feb 2024 11:45:16 +0100 Subject: [PATCH 2/2] doc: changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 27caa3ffb0..853579e6b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Fix regression in SQL query scrubbing. ([#3091](https://github.com/getsentry/relay/pull/3091)) - Normalize route in trace context data field. ([#3104](https://github.com/getsentry/relay/pull/3104)) +- Limit the length of scrubbed span descriptions. ([#3115](https://github.com/getsentry/relay/pull/3115)) **Features**: