fix(server): Enforce size limits on UE4 crash reports (#1099)

UE4 crash reports are compressed archives that are unpacked during
ingestion in Relay. After expansion, the size of the resulting envelope
and its file attachments was never checked, which could pass attachments
far exceeding the maximum size limits.

CHANGELOG.md

@@ -7,6 +7,7 @@
 - Correctly validate timestamps for outcomes and sessions. ([#1086](https://github.com/getsentry/relay/pull/1086))
 - Run compression on a thread pool when sending to upstream. ([#1085](https://github.com/getsentry/relay/pull/1085))
 - Report proper status codes and error messages when sending invalid JSON payloads to an endpoint with a `X-Sentry-Relay-Signature` header. ([#1090](https://github.com/getsentry/relay/pull/1090))
+- Enforce attachment and event size limits on UE4 crash reports. ([#1099](https://github.com/getsentry/relay/pull/1099))
 
 **Internal**:
 

relay-server/src/actors/envelopes.rs

@@ -934,6 +934,10 @@ impl EnvelopeProcessor {
     /// envelope contains an `UnrealReport` item, it removes it from the envelope and inserts new
     /// items for each of its contents.
     ///
+    /// The envelope may be dropped if it exceeds size limits after decompression. Particularly,
+    /// this includes cases where a single attachment file exceeds the maximum file size. This is in
+    /// line with the behavior of the envelope endpoint.
+    ///
     /// After this, the `EnvelopeProcessor` should be able to process the envelope the same way it
     /// processes any other envelopes.
     #[cfg(feature = "processing")]
@@ -943,6 +947,10 @@ impl EnvelopeProcessor {
         if let Some(item) = envelope.take_item_by(|item| item.ty() == ItemType::UnrealReport) {
             utils::expand_unreal_envelope(item, envelope)
                 .map_err(ProcessingError::InvalidUnrealReport)?;
+
+            if !utils::check_envelope_size_limits(&self.config, envelope) {
+                return Err(ProcessingError::PayloadTooLarge);
+            }
         }
 
         Ok(())

relay-server/src/endpoints/common.rs

@@ -12,7 +12,6 @@ use futures::prelude::*;
 use serde::Deserialize;
 
 use relay_common::{clone, tryf};
-use relay_config::Config;
 use relay_general::protocol::{EventId, EventType};
 use relay_log::LogError;
 use relay_quotas::RateLimits;
@@ -314,53 +313,6 @@ pub fn cors(app: ServiceApp) -> CorsBuilder<ServiceState> {
     builder
 }
 
-/// Checks for size limits of items in this envelope.
-///
-/// Returns `true`, if the envelope adheres to the configured size limits. Otherwise, returns
-/// `false`, in which case the envelope should be discarded and a `413 Payload Too Large` response
-/// shoult be given.
-///
-/// The following limits are checked:
-///
-///  - `max_event_size`
-///  - `max_attachment_size`
-///  - `max_attachments_size`
-///  - `max_session_count`
-fn check_envelope_size_limits(config: &Config, envelope: &Envelope) -> bool {
-    let mut event_size = 0;
-    let mut attachments_size = 0;
-    let mut session_count = 0;
-    let mut client_reports_size = 0;
-
-    for item in envelope.items() {
-        match item.ty() {
-            ItemType::Event
-            | ItemType::Transaction
-            | ItemType::Security
-            | ItemType::RawSecurity
-            | ItemType::FormData => event_size += item.len(),
-            ItemType::Attachment | ItemType::UnrealReport => {
-                if item.len() > config.max_attachment_size() {
-                    return false;
-                }
-
-                attachments_size += item.len()
-            }
-            ItemType::Session => session_count += 1,
-            ItemType::Sessions => session_count += 1,
-            ItemType::UserReport => (),
-            ItemType::Metrics => (),
-            ItemType::MetricBuckets => (),
-            ItemType::ClientReport => client_reports_size += item.len(),
-        }
-    }
-
-    event_size <= config.max_event_size()
-        && attachments_size <= config.max_attachments_size()
-        && session_count <= config.max_session_count()
-        && client_reports_size <= config.max_client_reports_size()
-}
-
 /// Handles Sentry events.
 ///
 /// Sentry events may come either directly from a http request ( the store endpoint calls this
@@ -442,7 +394,7 @@ where
             };
 
             envelope_context.update(&envelope);
-            if check_envelope_size_limits(&config, &envelope) {
+            if utils::check_envelope_size_limits(&config, &envelope) {
                 Ok((envelope, checked.rate_limits))
             } else {
                 envelope_context.send_outcomes(Outcome::Invalid(DiscardReason::TooLarge));

relay-server/src/utils/mod.rs

@@ -7,6 +7,7 @@ mod param_parser;
 mod rate_limits;
 mod request;
 mod shutdown;
+mod sizes;
 mod timer;
 mod tracked_future;
 mod with_outcome;
@@ -27,6 +28,7 @@ pub use self::param_parser::*;
 pub use self::rate_limits::*;
 pub use self::request::*;
 pub use self::shutdown::*;
+pub use self::sizes::*;
 pub use self::timer::*;
 pub use self::tracked_future::*;
 pub use self::with_outcome::*;

relay-server/src/utils/sizes.rs

@@ -0,0 +1,50 @@
+use relay_config::Config;
+
+use crate::envelope::{Envelope, ItemType};
+
+/// Checks for size limits of items in this envelope.
+///
+/// Returns `true`, if the envelope adheres to the configured size limits. Otherwise, returns
+/// `false`, in which case the envelope should be discarded and a `413 Payload Too Large` response
+/// should be given.
+///
+/// The following limits are checked:
+///
+///  - `max_event_size`
+///  - `max_attachment_size`
+///  - `max_attachments_size`
+///  - `max_session_count`
+pub fn check_envelope_size_limits(config: &Config, envelope: &Envelope) -> bool {
+    let mut event_size = 0;
+    let mut attachments_size = 0;
+    let mut session_count = 0;
+    let mut client_reports_size = 0;
+
+    for item in envelope.items() {
+        match item.ty() {
+            ItemType::Event
+            | ItemType::Transaction
+            | ItemType::Security
+            | ItemType::RawSecurity
+            | ItemType::FormData => event_size += item.len(),
+            ItemType::Attachment | ItemType::UnrealReport => {
+                if item.len() > config.max_attachment_size() {
+                    return false;
+                }
+
+                attachments_size += item.len()
+            }
+            ItemType::Session => session_count += 1,
+            ItemType::Sessions => session_count += 1,
+            ItemType::UserReport => (),
+            ItemType::Metrics => (),
+            ItemType::MetricBuckets => (),
+            ItemType::ClientReport => client_reports_size += item.len(),
+        }
+    }
+
+    event_size <= config.max_event_size()
+        && attachments_size <= config.max_attachments_size()
+        && session_count <= config.max_session_count()
+        && client_reports_size <= config.max_client_reports_size()
+}

tests/integration/test_unreal.py

@@ -1,6 +1,7 @@
 import os
 import pytest
 import json
+import time
 
 
 def _load_dump_file(base_file_name: str):
@@ -313,3 +314,25 @@ def test_unreal_minidump_with_config_and_processing(
             mini_dump_process_marker_found = True
 
     assert mini_dump_process_marker_found
+
+
+def test_unreal_crash_too_large(mini_sentry, relay_with_processing, outcomes_consumer):
+    PROJECT_ID = 42
+    unreal_content = _load_dump_file("unreal_crash")
+    print("UE4 size: %s" % len(unreal_content))
+
+    # Configure Relay so that it accepts the compressed UE4 archive. Once uncompressed, the file
+    # attachments are larger and should be dropped.
+    relay = relay_with_processing(
+        {"limits": {"max_attachments_size": len(unreal_content) + 1}}
+    )
+    mini_sentry.add_full_project_config(PROJECT_ID)
+    outcomes_consumer = outcomes_consumer()
+
+    # Relay accepts the archive, expands it asynchronously, and then drops it.
+    response = relay.send_unreal_request(PROJECT_ID, unreal_content)
+    assert response.ok
+
+    outcome = outcomes_consumer.get_outcome()
+    assert outcome["outcome"] == 3  # dropped as invalid
+    assert mini_sentry.captured_events.empty()