-
Notifications
You must be signed in to change notification settings - Fork 120
fix(relay): Limit maximum number of otel logs deserialized from JSON #6273
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,11 +7,16 @@ use crate::processing::logs::{Error, Result, Settings}; | |
| use crate::services::outcome::DiscardReason; | ||
|
|
||
| /// Expands OTeL logs into the [`OurLog`] format. | ||
| pub fn expand<F>(format: OtelFormat, payload: &[u8], mut produce: F) -> Result<Settings> | ||
| pub fn expand<F>( | ||
| format: OtelFormat, | ||
| payload: &[u8], | ||
| max_ops: usize, | ||
| mut produce: F, | ||
| ) -> Result<Settings> | ||
| where | ||
| F: FnMut(OurLog), | ||
| { | ||
| let logs = parse_logs_data(format, payload)?; | ||
| let logs = parse_logs_data(format, payload, max_ops)?; | ||
|
|
||
| for resource_logs in logs.resource_logs { | ||
| let resource = resource_logs.resource.as_ref(); | ||
|
|
@@ -27,15 +32,18 @@ where | |
| Ok(Settings::default()) | ||
| } | ||
|
|
||
| fn parse_logs_data(format: OtelFormat, payload: &[u8]) -> Result<LogsData, Error> { | ||
| fn parse_logs_data(format: OtelFormat, payload: &[u8], max_ops: usize) -> Result<LogsData, Error> { | ||
| match format { | ||
| OtelFormat::Json => serde_json::from_slice(payload).map_err(|e| { | ||
| relay_log::debug!( | ||
| error = &e as &dyn std::error::Error, | ||
| "Failed to parse logs data as JSON" | ||
| ); | ||
| Error::Invalid(DiscardReason::InvalidJson) | ||
| }), | ||
| OtelFormat::Json => { | ||
| let mut de = serde_json::Deserializer::from_reader(payload); | ||
| relay_serialization::serde::deserialize(&mut de, max_ops).map_err(|e| { | ||
| relay_log::debug!( | ||
| error = &e as &dyn std::error::Error, | ||
| "Failed to parse logs data as JSON" | ||
| ); | ||
| Error::Invalid(DiscardReason::InvalidJson) | ||
| }) | ||
|
Comment on lines
+39
to
+45
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Deserialization operation limit errors are incorrectly classified as Suggested FixUpdate the Prompt for AI Agent |
||
| } | ||
| OtelFormat::Protobuf => LogsData::decode(payload).map_err(|e| { | ||
| relay_log::debug!( | ||
| error = &e as &dyn std::error::Error, | ||
|
|
@@ -45,3 +53,109 @@ fn parse_logs_data(format: OtelFormat, payload: &[u8]) -> Result<LogsData, Error | |
| }), | ||
| } | ||
| } | ||
| #[cfg(test)] | ||
| mod tests { | ||
|
|
||
| use opentelemetry_proto::tonic::common::v1::any_value::Value; | ||
| use opentelemetry_proto::tonic::common::v1::{ | ||
| AnyValue, ArrayValue, InstrumentationScope, KeyValue, | ||
| }; | ||
| use opentelemetry_proto::tonic::resource::v1::Resource; | ||
| use relay_ourlogs::otel_logs::{LogRecord, LogsData, ResourceLogs, ScopeLogs}; | ||
|
|
||
| use crate::processing::logs::integrations::otel::parse_logs_data; | ||
|
|
||
| #[test] | ||
| fn test_basic_json() { | ||
| let log_data = LogsData { | ||
| resource_logs: vec![ResourceLogs { | ||
| resource: Some(Resource { | ||
| attributes: vec![KeyValue { | ||
| key: "service.name".to_owned(), | ||
| value: Some(AnyValue { | ||
| value: Some(Value::StringValue("test-service".to_owned())), | ||
| }), | ||
| }], | ||
| dropped_attributes_count: 0, | ||
| entity_refs: vec![], | ||
| }), | ||
| scope_logs: vec![ScopeLogs { | ||
| scope: Some(InstrumentationScope { | ||
| name: "test-library".to_owned(), | ||
| version: "".to_owned(), | ||
| attributes: vec![], | ||
| dropped_attributes_count: 0, | ||
| }), | ||
| log_records: vec![LogRecord { | ||
| time_unix_nano: 123, | ||
| observed_time_unix_nano: 123, | ||
| severity_number: 2, | ||
| severity_text: "Information".to_owned(), | ||
| body: Some(AnyValue { | ||
| value: Some(Value::StringValue("a body".to_owned())), | ||
| }), | ||
| attributes: vec![ | ||
| KeyValue { | ||
| key: "attribute".to_owned(), | ||
| value: Some(AnyValue { | ||
| value: Some(Value::StringValue("value".to_owned())), | ||
| }), | ||
| }, | ||
| KeyValue { | ||
| key: "nested attribute".to_owned(), | ||
| value: Some(AnyValue { | ||
| value: Some(Value::ArrayValue(ArrayValue { | ||
| values: vec![AnyValue { | ||
| value: Some(Value::StringValue("value".to_owned())), | ||
| }], | ||
| })), | ||
| }), | ||
| }, | ||
| ], | ||
| dropped_attributes_count: 0, | ||
| flags: 0, | ||
| trace_id: "5B8EFFF798038103D269B633813FC60C".into(), | ||
| span_id: "EEE19B7EC3C1B174".into(), | ||
| event_name: "".to_owned(), | ||
| }], | ||
| schema_url: "".to_owned(), | ||
| }], | ||
| schema_url: "http://example.com".to_owned(), | ||
| }], | ||
| }; | ||
|
|
||
| let json = serde_json::to_string(&log_data).unwrap(); | ||
|
|
||
| let unjson: LogsData = serde_json::from_str(&json).unwrap(); | ||
|
|
||
| assert_eq!(unjson, log_data); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_abusive_json() { | ||
| let mut abusive_log = "{},".repeat(1_001); | ||
| abusive_log.pop(); | ||
|
|
||
| let json = r#"{ | ||
| "resourceLogs": [ | ||
| { | ||
| "resource": { | ||
| "attributes": [ | ||
| { | ||
| "key": "service.name", | ||
| "value": {"stringValue": "test-service"} | ||
| } | ||
| ] | ||
| }, | ||
| "scopeLogs": [ | ||
| { | ||
| "scope": {"name": "test-library"}, | ||
| "logRecords": ["# | ||
| .to_owned(); | ||
| let json = json + &abusive_log + "]}]}]}"; | ||
|
|
||
| assert!( | ||
| parse_logs_data(crate::integrations::OtelFormat::Json, json.as_bytes(), 1000).is_err() | ||
| ); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OTel protobuf log expansion ignores max_expanded_log_count limit
otel::expandreceivesmax_expanded_log_countfor both JSON and protobuf, but only applies it to JSON, allowing a protobuf payload with millions of minimal log records to bypass the limit and exhaust CPU.Evidence
mod.rs:49passesmax_expanded_log_counttootel::expand(format, &payload, max_expanded_log_count, produce).otel.rs:36parse_logs_data(format, payload, max_logs)receives the limit asmax_logs.otel.rs:45usesLogsData::decode(payload)forOtelFormat::Protobuf, silently discardingmax_logs.otel.rs:22-31iterates every decoded record and callsproducefor each, so a protobuf with millions of minimal records expands to millions of logs.LogRecordentries, each processed throughrelay_ourlogs::otel_to_sentry_log.Also found at 6 additional locations
relay-config/src/config.rs:642-642relay-config/src/config.rs:733relay-server/src/processing/logs/integrations/otel.rs:45-50relay-server/src/processing/logs/mod.rs:58relay-server/src/processing/logs/process.rs:50-51relay-server/src/processing/logs/mod.rs:161Identified by Warden · wrdn-dos-review · U83-KKN