Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5064,6 +5064,7 @@ dependencies = [
"relay-redis",
"relay-replays",
"relay-sampling",
"relay-serialization",
"relay-spans",
"relay-statsd",
"relay-system",
Expand Down
8 changes: 8 additions & 0 deletions relay-config/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -645,6 +645,8 @@ pub struct Limits {
pub max_trace_metric_size: ByteSize,
/// The maximum payload size for a log.
pub max_log_size: ByteSize,
/// The maximum number of operations that can occur in a log expansion.
pub max_expanded_log_operations: usize,
/// The maximum payload size for a span.
pub max_span_size: ByteSize,
/// The maximum amount of standalone transaction spans per envelope.
Expand Down Expand Up @@ -739,6 +741,7 @@ impl Default for Limits {
max_profile_size: ByteSize::mebibytes(50),
max_trace_metric_size: ByteSize::mebibytes(1),
max_log_size: ByteSize::mebibytes(2),
max_expanded_log_operations: 2_000_000,
max_span_size: ByteSize::mebibytes(10),
max_standalone_span_count: 25,
max_container_size: ByteSize::mebibytes(12),
Expand Down Expand Up @@ -2434,6 +2437,11 @@ impl Config {
self.values.limits.max_log_size.as_bytes()
}

/// Returns the maximum number of operations to allow for a log expansion.
pub fn max_expanded_log_operations(&self) -> usize {
self.values.limits.max_expanded_log_operations
}

/// Returns the maximum payload size of a span in bytes.
pub fn max_span_size(&self) -> usize {
self.values.limits.max_span_size.as_bytes()
Expand Down
1 change: 1 addition & 0 deletions relay-server/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ relay-redis = { workspace = true }
relay-replays = { workspace = true }
relay-conventions = { workspace = true }
relay-sampling = { workspace = true }
relay-serialization = { workspace = true }
relay-spans = { workspace = true }
relay-statsd = { workspace = true }
relay-system = { workspace = true }
Expand Down
3 changes: 2 additions & 1 deletion relay-server/src/processing/logs/integrations/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
item: Item,
records: &mut RecordKeeper<'_>,
headers: &EnvelopeHeaders,
max_ops: usize,
) -> Option<(Settings, ContainerItems<OurLog>)> {
let integration = match item.integration() {
Some(Integration::Logs(integration)) => integration,
Expand Down Expand Up @@ -45,9 +46,9 @@
let payload = item.payload();

let settings = match integration {

Copy link
Copy Markdown

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::expand receives max_expanded_log_count for 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:49 passes max_expanded_log_count to otel::expand(format, &payload, max_expanded_log_count, produce).
  • otel.rs:36 parse_logs_data(format, payload, max_logs) receives the limit as max_logs.
  • otel.rs:45 uses LogsData::decode(payload) for OtelFormat::Protobuf, silently discarding max_logs.
  • otel.rs:22-31 iterates every decoded record and calls produce for each, so a protobuf with millions of minimal records expands to millions of logs.
  • A crafted protobuf near the max envelope size (200 MiB) could contain millions of tiny LogRecord entries, each processed through relay_ourlogs::otel_to_sentry_log.
Also found at 6 additional locations
  • relay-config/src/config.rs:642-642
  • relay-config/src/config.rs:733
  • relay-server/src/processing/logs/integrations/otel.rs:45-50
  • relay-server/src/processing/logs/mod.rs:58
  • relay-server/src/processing/logs/process.rs:50-51
  • relay-server/src/processing/logs/mod.rs:161

Identified by Warden · wrdn-dos-review · U83-KKN

LogsIntegration::Nel => nel::expand(&payload, headers, produce),
LogsIntegration::OtelV1 { format } => otel::expand(format, &payload, produce),
LogsIntegration::OtelV1 { format } => otel::expand(format, &payload, max_ops, produce),
LogsIntegration::VercelDrainLog { format } => vercel::expand(format, &payload, produce),

Check failure on line 51 in relay-server/src/processing/logs/integrations/mod.rs

View check run for this annotation

@sentry/warden / warden: wrdn-dos-review

[46F-QC4] Non-OTEL-JSON log expansion lacks an operation or record-count bound (additional location)

Attacker-controlled NEL, Vercel, log-container, and OTEL protobuf payloads are deserialized and expanded without the configured operation budget; only OTEL JSON uses the metered deserializer. The 12 MiB payload cap and post-expansion per-log size validation do not prevent millions of tiny records from being materialized and retained, allowing a single request to cause disproportionate memory and CPU use in the core ingestion path.
};
let settings = match settings {
Ok(settings) => settings,
Expand Down
134 changes: 124 additions & 10 deletions relay-server/src/processing/logs/integrations/otel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Deserialization operation limit errors are incorrectly classified as InvalidJson, conflating resource exhaustion with malformed data and impacting observability.
Severity: MEDIUM

Suggested Fix

Update the map_err closure to differentiate between error variants from relay_serialization::serde::deserialize. Match on the error and map LimitExceeded to a new, specific DiscardReason (e.g., Complexity) and Serde errors to InvalidJson. This will likely require adding a new variant to the DiscardReason enum.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: relay-server/src/processing/logs/integrations/otel.rs#L39-L45

Potential issue: In the OTel JSON log processing, the error handling for deserialization
does not distinguish between different failure modes. The
`relay_serialization::serde::deserialize` function can fail with a `LimitExceeded` error
if the JSON is too complex, or a `Serde` error for malformed JSON. The current
implementation in `otel.rs` maps both error types to `DiscardReason::InvalidJson`. This
misclassifies payloads that exceed the operation budget as having invalid JSON, which
hinders observability and makes it difficult to detect and monitor abuse attempts
involving overly complex payloads.

}
OtelFormat::Protobuf => LogsData::decode(payload).map_err(|e| {
relay_log::debug!(
error = &e as &dyn std::error::Error,
Expand All @@ -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()
);
}
}
2 changes: 1 addition & 1 deletion relay-server/src/processing/logs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@
// Fast filters, which do not need expanded logs.
filter::feature_flag(ctx).reject(&logs)?;

let mut logs = process::expand(logs)?;
let mut logs = process::expand(logs, ctx.config.max_expanded_log_operations())?;

Check failure on line 160 in relay-server/src/processing/logs/mod.rs

View check run for this annotation

@sentry/warden / warden: wrdn-dos-review

[46F-QC4] Non-OTEL-JSON log expansion lacks an operation or record-count bound (additional location)

Attacker-controlled NEL, Vercel, log-container, and OTEL protobuf payloads are deserialized and expanded without the configured operation budget; only OTEL JSON uses the metered deserializer. The 12 MiB payload cap and post-expansion per-log size validation do not prevent millions of tiny records from being materialized and retained, allowing a single request to cause disproportionate memory and CPU use in the core ingestion path.

validate::size(&mut logs, ctx);

Expand Down
7 changes: 5 additions & 2 deletions relay-server/src/processing/logs/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@
/// Parses all serialized logs into their [`ExpandedLogs`] representation.
///
/// Individual, invalid logs will be discarded.
pub fn expand(logs: Managed<SerializedLogs>) -> Result<Managed<ExpandedLogs>, Rejected<Error>> {
pub fn expand(
logs: Managed<SerializedLogs>,
max_ops: usize,
) -> Result<Managed<ExpandedLogs>, Rejected<Error>> {
let trust = logs.headers.meta().request_trust();

logs.try_map(|logs, records| {
Expand All @@ -41,11 +44,11 @@
LogItems::Integration(_) => Ingress::Integration,
};

let (settings, logs) = match items {
LogItems::Container(item) => expand_log_container(&item, trust)?,
LogItems::Integration(item) => {
logs::integrations::expand(item, records, &headers).unwrap_or_default()
logs::integrations::expand(item, records, &headers, max_ops).unwrap_or_default()
}

Check failure on line 51 in relay-server/src/processing/logs/process.rs

View check run for this annotation

@sentry/warden / warden: wrdn-dos-review

Non-OTEL-JSON log expansion lacks an operation or record-count bound

Attacker-controlled NEL, Vercel, log-container, and OTEL protobuf payloads are deserialized and expanded without the configured operation budget; only OTEL JSON uses the metered deserializer. The 12 MiB payload cap and post-expansion per-log size validation do not prevent millions of tiny records from being materialized and retained, allowing a single request to cause disproportionate memory and CPU use in the core ingestion path.
};

Ok::<_, Error>(ExpandedLogs {
Expand Down
Loading