diff --git a/CHANGELOG.md b/CHANGELOG.md index 847fd18171..f7c4b39295 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Add support for X-Vercel-Forwarded-For header. ([#2124](https://github.com/getsentry/relay/pull/2124)) - Add `lock` attribute to the frame protocol. ([#2171](https://github.com/getsentry/relay/pull/2171)) +- Reject profiles longer than 30s. ([#2168](https://github.com/getsentry/relay/pull/2168)) ## 23.5.2 diff --git a/relay-profiling/src/android.rs b/relay-profiling/src/android.rs index b8201f1ecf..35f9d36258 100644 --- a/relay-profiling/src/android.rs +++ b/relay-profiling/src/android.rs @@ -9,7 +9,7 @@ use serde::{Deserialize, Serialize}; use crate::measurements::Measurement; use crate::transaction_metadata::TransactionMetadata; use crate::utils::{deserialize_number_from_string, is_zero}; -use crate::ProfileError; +use crate::{ProfileError, MAX_PROFILE_DURATION}; #[derive(Debug, Serialize, Deserialize, Clone)] pub struct ProfileMetadata { @@ -183,6 +183,10 @@ fn parse_profile(payload: &[u8]) -> Result { return Err(ProfileError::NotEnoughSamples); } + if profile.profile.elapsed_time > MAX_PROFILE_DURATION { + return Err(ProfileError::DurationIsTooLong); + } + Ok(profile) } diff --git a/relay-profiling/src/error.rs b/relay-profiling/src/error.rs index 593a2adf03..09d1994b14 100644 --- a/relay-profiling/src/error.rs +++ b/relay-profiling/src/error.rs @@ -26,4 +26,6 @@ pub enum ProfileError { MalformedSamples, #[error("exceed size limit")] ExceedSizeLimit, + #[error("duration is too long")] + DurationIsTooLong, } diff --git a/relay-profiling/src/lib.rs b/relay-profiling/src/lib.rs index 8570365a03..3f715ff845 100644 --- a/relay-profiling/src/lib.rs +++ b/relay-profiling/src/lib.rs @@ -95,6 +95,7 @@ use serde::Deserialize; use std::collections::BTreeMap; +use std::time::Duration; mod android; mod cocoa; @@ -114,6 +115,8 @@ use crate::extract_from_transaction::{extract_transaction_metadata, extract_tran pub use crate::error::ProfileError; pub use crate::outcomes::discard_reason; +const MAX_PROFILE_DURATION: Duration = Duration::from_secs(30); + #[derive(Debug, Deserialize)] struct MinimalProfile { #[serde(alias = "profile_id")] diff --git a/relay-profiling/src/outcomes.rs b/relay-profiling/src/outcomes.rs index fa3fab9b0e..12d957fae3 100644 --- a/relay-profiling/src/outcomes.rs +++ b/relay-profiling/src/outcomes.rs @@ -14,5 +14,6 @@ pub fn discard_reason(err: ProfileError) -> &'static str { ProfileError::NoTransactionAssociated => "profiling_no_transaction_associated", ProfileError::NotEnoughSamples => "profiling_not_enough_samples", ProfileError::PlatformNotSupported => "profiling_platform_not_supported", + ProfileError::DurationIsTooLong => "profiling_duration_is_too_long", } } diff --git a/relay-profiling/src/sample.rs b/relay-profiling/src/sample.rs index 850c8439ae..2de7d2171f 100644 --- a/relay-profiling/src/sample.rs +++ b/relay-profiling/src/sample.rs @@ -9,6 +9,9 @@ use crate::measurements::Measurement; use crate::native_debug_image::NativeDebugImage; use crate::transaction_metadata::TransactionMetadata; use crate::utils::deserialize_number_from_string; +use crate::MAX_PROFILE_DURATION; + +const MAX_PROFILE_DURATION_NS: u64 = MAX_PROFILE_DURATION.as_nanos() as u64; #[derive(Debug, Serialize, Deserialize, Clone)] struct Frame { @@ -213,6 +216,13 @@ impl SampleProfile { true } + fn is_above_max_duration(&self) -> bool { + if let Some(sample) = &self.profile.samples.last() { + return sample.elapsed_since_start_ns > MAX_PROFILE_DURATION_NS; + } + false + } + /// Removes a sample when it's the only sample on its thread fn remove_single_samples_per_thread(&mut self) { let mut sample_count_by_thread_id: HashMap = HashMap::new(); @@ -327,6 +337,10 @@ fn parse_profile(payload: &[u8]) -> Result { return Err(ProfileError::MalformedStacks); } + if profile.is_above_max_duration() { + return Err(ProfileError::DurationIsTooLong); + } + profile.strip_pointer_authentication_code(); profile.cleanup_thread_metadata(); profile.cleanup_queue_metadata(); @@ -368,6 +382,7 @@ pub fn parse_sample_profile( #[cfg(test)] mod tests { use super::*; + use std::time::Duration; #[test] fn test_roundtrip() { @@ -909,4 +924,48 @@ mod tests { "some-random-transaction".to_string() ); } + + #[test] + fn test_keep_profile_under_max_duration() { + let mut profile = generate_profile(); + profile.profile.samples.extend(vec![ + Sample { + stack_id: 0, + queue_address: Some("0xdeadbeef".to_string()), + elapsed_since_start_ns: 10, + thread_id: 1, + }, + Sample { + stack_id: 0, + queue_address: Some("0xdeadbeef".to_string()), + elapsed_since_start_ns: (MAX_PROFILE_DURATION - Duration::from_secs(1)).as_nanos() + as u64, + thread_id: 2, + }, + ]); + + assert!(!profile.is_above_max_duration()); + } + + #[test] + fn test_reject_profile_over_max_duration() { + let mut profile = generate_profile(); + profile.profile.samples.extend(vec![ + Sample { + stack_id: 0, + queue_address: Some("0xdeadbeef".to_string()), + elapsed_since_start_ns: 10, + thread_id: 1, + }, + Sample { + stack_id: 0, + queue_address: Some("0xdeadbeef".to_string()), + elapsed_since_start_ns: (MAX_PROFILE_DURATION + Duration::from_secs(1)).as_nanos() + as u64, + thread_id: 2, + }, + ]); + + assert!(profile.is_above_max_duration()); + } }