fix: Limit form data chunk index

CHANGELOG.md

@@ -18,6 +18,7 @@
 - Set sentry.trace.status on segment spans. ([#6140](https://github.com/getsentry/relay/pull/6140))
 - Don't modify segment information for V2 web vital spans. ([#6160](https://github.com/getsentry/relay/pull/6160))
 - Support compressed minidumps when the `relay-minidump-uploads` feature is enabled. ([#6151](https://github.com/getsentry/relay/pull/6151))
+- Limit indexes of event form-data chunks. ([#6173](https://github.com/getsentry/relay/pull/6173))
 
 **Internal**:
 

relay-server/src/processing/errors/errors/utils/formdata.rs

@@ -1,9 +1,10 @@
+use relay_config::Config;
 use serde_json::Value as SerdeValue;
 
 use crate::envelope::Item;
 use crate::utils::{self, ChunkedFormDataAggregator, FormDataIter};
 
-pub fn merge_formdata(target: &mut SerdeValue, item: &Item) {
+pub fn merge_formdata(target: &mut SerdeValue, item: &Item, config: &Config) {
     let payload = item.payload();
     let mut aggregator = ChunkedFormDataAggregator::new();
 
@@ -15,7 +16,7 @@ pub fn merge_formdata(target: &mut SerdeValue, item: &Item) {
                 Ok(event) => utils::merge_values(target, event),
                 Err(_) => relay_log::debug!("invalid json event payload in sentry form field"),
             }
-        } else if let Some(index) = utils::get_sentry_chunk_index(entry.key(), "sentry__") {
+        } else if let Some(index) = utils::get_sentry_chunk_index(entry.key(), "sentry__", config) {
             // Electron SDK splits up long payloads into chunks starting at sentry__1 with an
             // incrementing counter. Assemble these chunks here and then decode them below.
             aggregator.insert(index, entry.value());
@@ -39,3 +40,43 @@ pub fn merge_formdata(target: &mut SerdeValue, item: &Item) {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::envelope::{ContentType, ItemType};
+    use crate::utils::FormDataWriter;
+
+    fn form_data_item(fields: &[(&str, &str)]) -> Item {
+        let mut writer = FormDataWriter::new();
+        for (key, value) in fields {
+            writer.append(key, value);
+        }
+        let mut item = Item::new(ItemType::FormData);
+        item.set_payload(ContentType::Text, writer.into_inner());
+        item
+    }
+
+    #[test]
+    fn test_merge_formdata_assembles_chunks() {
+        let item = form_data_item(&[("sentry__0", r#"{"message":"#), ("sentry__1", r#""hi"}"#)]);
+        let mut target = SerdeValue::Object(Default::default());
+
+        merge_formdata(&mut target, &item, &Config::default());
+
+        assert_eq!(target, serde_json::json!({ "message": "hi" }));
+    }
+
+    #[test]
+    fn test_merge_formdata_ignores_too_large_index() {
+        let item = form_data_item(&[("sentry__2000", "x")]);
+        let mut target = SerdeValue::Object(Default::default());
+
+        merge_formdata(&mut target, &item, &Config::default());
+
+        assert_eq!(
+            target,
+            serde_json::json!({ "extra": { "sentry__2000": "x" } })
+        );
+    }
+}

relay-server/src/processing/errors/errors/utils/mod.rs

@@ -158,14 +158,15 @@ pub fn take_event_from_attachments(
 pub fn take_event_from_formdata(
     items: &mut Vec<Item>,
     metrics: &mut Metrics,
+    ctx: Context<'_>,
 ) -> Result<Annotated<Event>> {
     let Some(form_data) = take_item_of_type(items, ItemType::FormData) else {
         return Ok(Annotated::empty());
     };
 
     let event = {
         let mut value = serde_json::Value::Object(Default::default());
-        formdata::merge_formdata(&mut value, &form_data);
+        formdata::merge_formdata(&mut value, &form_data, ctx.processing.config);
         Annotated::deserialize_with_meta(value).map_err(ProcessingError::InvalidJson)
     }?;
     metrics.bytes_ingested_event = Annotated::new(form_data.len() as u64);
@@ -199,7 +200,7 @@ pub fn take_event_from_crash_items(
         return Ok(event);
     }
 
-    let event = take_event_from_formdata(items, metrics)?;
+    let event = take_event_from_formdata(items, metrics, ctx)?;
     if event.0.is_some() {
         return Ok(event);
     }

relay-server/src/utils/multipart.rs

@@ -94,7 +94,7 @@ impl<'a> FormDataEntry<'a> {
 ///
 /// This writer is used to serialize multiple plain fields from a multipart form data request into a
 /// single envelope item. Use `FormDataIter` to iterate all entries.
-struct FormDataWriter {
+pub struct FormDataWriter {
     data: Vec<u8>,
 }
 

relay-server/src/utils/param_parser.rs

@@ -1,3 +1,4 @@
+use relay_config::Config;
 use serde_json::Value;
 
 enum IndexingState {
@@ -98,8 +99,12 @@ pub fn get_sentry_entry_indexes(param_name: &str) -> Option<Vec<&str>> {
 ///
 /// Electron SDK splits up long payloads into chunks starting at sentry__1 with an
 /// incrementing counter. Assemble these chunks here and then decode them below.
-pub fn get_sentry_chunk_index(key: &str, prefix: &str) -> Option<usize> {
-    key.strip_prefix(prefix).and_then(|rest| rest.parse().ok())
+///
+/// If the index is too large, or unparsable from `key`, `None` is returned.
+pub fn get_sentry_chunk_index(key: &str, prefix: &str, config: &Config) -> Option<usize> {
+    key.strip_prefix(prefix)
+        .and_then(|rest| rest.parse().ok())
+        .filter(|&index| index <= config.max_event_size() / 1024)
 }
 
 /// Aggregates slices of strings in random order.
@@ -239,13 +244,42 @@ mod tests {
 
     #[test]
     fn test_chunk_index() {
-        assert_eq!(get_sentry_chunk_index("sentry__0", "sentry__"), Some(0));
-        assert_eq!(get_sentry_chunk_index("sentry__1", "sentry__"), Some(1));
+        let config = Config::default();
+        assert_eq!(
+            get_sentry_chunk_index("sentry__0", "sentry__", &config),
+            Some(0)
+        );
+        assert_eq!(
+            get_sentry_chunk_index("sentry__1", "sentry__", &config),
+            Some(1)
+        );
+
+        assert_eq!(get_sentry_chunk_index("foo__0", "sentry__", &config), None);
+        assert_eq!(
+            get_sentry_chunk_index("sentry__", "sentry__", &config),
+            None
+        );
+        assert_eq!(
+            get_sentry_chunk_index("sentry__-1", "sentry__", &config),
+            None
+        );
+        assert_eq!(
+            get_sentry_chunk_index("sentry__xx", "sentry__", &config),
+            None
+        );
+    }
 
-        assert_eq!(get_sentry_chunk_index("foo__0", "sentry__"), None);
-        assert_eq!(get_sentry_chunk_index("sentry__", "sentry__"), None);
-        assert_eq!(get_sentry_chunk_index("sentry__-1", "sentry__"), None);
-        assert_eq!(get_sentry_chunk_index("sentry__xx", "sentry__"), None);
+    #[test]
+    fn test_chunk_index_derived_from_max_event_size() {
+        let config = Config::default();
+        assert_eq!(
+            get_sentry_chunk_index("sentry__1024", "sentry__", &config),
+            Some(1024)
+        );
+        assert_eq!(
+            get_sentry_chunk_index("sentry__1025", "sentry__", &config),
+            None
+        );
     }
 
     #[test]