diff --git a/relay-server/src/endpoints/minidump.rs b/relay-server/src/endpoints/minidump.rs index e1e1143915..11db4147ef 100644 --- a/relay-server/src/endpoints/minidump.rs +++ b/relay-server/src/endpoints/minidump.rs @@ -22,6 +22,7 @@ use crate::envelope::{AttachmentType, Envelope, Item, ItemType}; use crate::extractors::{RawContentType, RequestMeta}; use crate::middlewares; use crate::service::ServiceState; +use crate::services::outcome::{DiscardAttachmentType, DiscardItemType}; use crate::utils::{self, ConstrainedMultipart}; /// The field name of a minidump in the multipart form-data upload. @@ -65,10 +66,22 @@ fn validate_minidump(data: &[u8]) -> Result<(), BadStoreRequest> { Ok(()) } -/// Convenience wrapper to let a decoder decode its full input into a buffer -fn run_decoder(decoder: &mut Box) -> std::io::Result> { +/// Convenience wrapper to let a decoder decode its full input into a buffer. +/// +/// Stops reading once `max_size` is exceeded and returns an error. This prevents +/// decompression bombs from exhausting memory. +fn run_decoder(decoder: Box, max_size: usize) -> std::io::Result> { let mut buffer = Vec::new(); - decoder.read_to_end(&mut buffer)?; + // Read up to max_size + 1 bytes to detect if the limit is exceeded + decoder + .take((max_size + 1) as u64) + .read_to_end(&mut buffer)?; + if buffer.len() > max_size { + return Err(std::io::Error::new( + std::io::ErrorKind::Other, + "decompressed content exceeds maximum attachment size", + )); + } Ok(buffer) } @@ -94,19 +107,26 @@ fn decoder_from(minidump_data: Bytes) -> Option> { } /// Tries to decode a minidump using any of the supported compression formats -/// or returns the provided minidump payload untouched if no format where detected -fn decode_minidump(minidump_data: Bytes) -> Result { +/// or returns the provided minidump payload untouched if no format where detected. +/// +/// Returns an `Overflow` error if the decompressed size exceeds `max_size`. +fn decode_minidump(minidump_data: Bytes, max_size: usize) -> Result { match decoder_from(minidump_data.clone()) { - Some(mut decoder) => { - match run_decoder(&mut decoder) { - Ok(decoded) => Ok(Bytes::from(decoded)), - Err(err) => { - // we detected a compression container but failed to decode it - relay_log::trace!("invalid compression container"); - Err(BadStoreRequest::InvalidCompressionContainer(err)) - } + Some(decoder) => match run_decoder(decoder, max_size) { + Ok(decoded) => Ok(Bytes::from(decoded)), + Err(err) if err.kind() == std::io::ErrorKind::Other => { + // Size limit exceeded during decompression + relay_log::trace!("decompressed minidump exceeds size limit"); + Err(BadStoreRequest::Overflow(DiscardItemType::Attachment( + DiscardAttachmentType::Minidump, + ))) } - } + Err(err) => { + // we detected a compression container but failed to decode it + relay_log::trace!("invalid compression container"); + Err(BadStoreRequest::InvalidCompressionContainer(err)) + } + }, None => { // this means we haven't detected any compression container // proceed to process the payload untouched (as a plain minidump). @@ -181,7 +201,10 @@ async fn extract_multipart( minidump_item.set_payload(Minidump, embedded); } - minidump_item.set_payload(Minidump, decode_minidump(minidump_item.payload())?); + minidump_item.set_payload( + Minidump, + decode_minidump(minidump_item.payload(), config.max_attachment_size())?, + ); validate_minidump(&minidump_item.payload())?; @@ -199,10 +222,14 @@ async fn extract_multipart( Ok(envelope) } -fn extract_raw_minidump(data: Bytes, meta: RequestMeta) -> Result, BadStoreRequest> { +fn extract_raw_minidump( + data: Bytes, + meta: RequestMeta, + max_size: usize, +) -> Result, BadStoreRequest> { let mut item = Item::new(ItemType::Attachment); - item.set_payload(Minidump, decode_minidump(data)?); + item.set_payload(Minidump, decode_minidump(data, max_size)?); validate_minidump(&item.payload())?; item.set_filename(MINIDUMP_FILE_NAME); item.set_attachment_type(AttachmentType::Minidump); @@ -224,11 +251,12 @@ async fn handle( // Minidump request payloads do not have the same structure as usual events from other SDKs. The // minidump can either be transmitted as request body, or as `upload_file_minidump` in a // multipart formdata request. + let config = state.config(); let envelope = if MINIDUMP_RAW_CONTENT_TYPES.contains(&content_type.as_ref()) { - extract_raw_minidump(request.extract().await?, meta)? + extract_raw_minidump(request.extract().await?, meta, config.max_attachment_size())? } else { let multipart = request.extract_with_state(&state).await?; - extract_multipart(multipart, meta, state.config()).await? + extract_multipart(multipart, meta, config).await? }; let id = envelope.event_id(); @@ -313,22 +341,23 @@ mod tests { #[test] fn test_validate_encoded_minidump() -> Result<(), Box> { let encoders: Vec = vec![encode_gzip, encode_zst, encode_bzip, encode_xz]; + let max_size = 1024 * 1024; // 1 MB, large enough for test data for encoder in &encoders { let be_minidump = b"PMDMxxxxxx"; let compressed = encoder(be_minidump)?; - let mut decoder = decoder_from(compressed).unwrap(); - assert!(run_decoder(&mut decoder).is_ok()); + let decoder = decoder_from(compressed).unwrap(); + assert!(run_decoder(decoder, max_size).is_ok()); let le_minidump = b"MDMPxxxxxx"; let compressed = encoder(le_minidump)?; - let mut decoder = decoder_from(compressed).unwrap(); - assert!(run_decoder(&mut decoder).is_ok()); + let decoder = decoder_from(compressed).unwrap(); + assert!(run_decoder(decoder, max_size).is_ok()); let garbage = b"xxxxxx"; let compressed = encoder(garbage)?; - let mut decoder = decoder_from(compressed).unwrap(); - let decoded = run_decoder(&mut decoder); + let decoder = decoder_from(compressed).unwrap(); + let decoded = run_decoder(decoder, max_size); assert!(decoded.is_ok()); assert!(validate_minidump(&decoded.unwrap()).is_err()); } @@ -336,6 +365,24 @@ mod tests { Ok(()) } + #[test] + fn test_decode_minidump_size_limit() -> Result<(), Box> { + // Create a minidump that will decompress to 100 bytes + let minidump_data = b"xxxxxxxxxx".repeat(10); + let compressed = encode_gzip(&minidump_data)?; + + // With a limit larger than the decompressed size, decoding should succeed + let result = decode_minidump(compressed.clone(), 200); + assert!(result.is_ok()); + assert_eq!(result.unwrap().len(), 100); + + // With a limit smaller than the decompressed size, decoding should fail with Overflow + let result = decode_minidump(compressed, 50); + assert!(matches!(result, Err(BadStoreRequest::Overflow(_)))); + + Ok(()) + } + #[test] fn test_remove_container_extension() -> Result<(), Box> { assert_eq!(remove_container_extension("minidump"), "minidump");