diff --git a/tracing-subscriber/CHANGELOG.md b/tracing-subscriber/CHANGELOG.md index bc3d20f7ae..91dd360d22 100644 --- a/tracing-subscriber/CHANGELOG.md +++ b/tracing-subscriber/CHANGELOG.md @@ -1,3 +1,11 @@ +# 0.3.20 (August 29, 2025) + +[ [crates.io][crate-0.3.20] ] | [ [docs.rs][docs-0.3.20] ] + +### Fixed + +- Escape ANSI escape sequences in logs + # 0.3.19 (November 29, 2024) [ [crates.io][crate-0.3.19] ] | [ [docs.rs][docs-0.3.19] ] diff --git a/tracing-subscriber/Cargo.toml b/tracing-subscriber/Cargo.toml index 1ea08eb70b..8184e2117a 100644 --- a/tracing-subscriber/Cargo.toml +++ b/tracing-subscriber/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "tracing-subscriber" -version = "0.3.19" +version = "0.3.20" authors = [ "Eliza Weisman ", "David Barsky ", diff --git a/tracing-subscriber/src/fmt/format/escape.rs b/tracing-subscriber/src/fmt/format/escape.rs new file mode 100644 index 0000000000..00837b04b3 --- /dev/null +++ b/tracing-subscriber/src/fmt/format/escape.rs @@ -0,0 +1,51 @@ +//! ANSI escape sequence sanitization to prevent terminal injection attacks. + +use std::fmt::{self, Write}; + +/// A wrapper that implements `fmt::Debug` and `fmt::Display` and escapes ANSI sequences on-the-fly. +/// This avoids creating intermediate strings while providing security against terminal injection. +pub(super) struct Escape(pub(super) T); + +/// Helper struct that escapes ANSI sequences as characters are written +struct EscapingWriter<'a, 'b> { + inner: &'a mut fmt::Formatter<'b>, +} + +impl<'a, 'b> fmt::Write for EscapingWriter<'a, 'b> { + fn write_str(&mut self, s: &str) -> fmt::Result { + // Stream the string character by character, escaping ANSI and C1 control sequences + for ch in s.chars() { + match ch { + // C0 control characters that can be used in terminal escape sequences + '\x1b' => self.inner.write_str("\\x1b")?, // ESC + '\x07' => self.inner.write_str("\\x07")?, // BEL + '\x08' => self.inner.write_str("\\x08")?, // BS + '\x0c' => self.inner.write_str("\\x0c")?, // FF + '\x7f' => self.inner.write_str("\\x7f")?, // DEL + + // C1 control characters (\x80-\x9f) - 8-bit control codes + // These can be used as alternative escape sequences in some terminals + ch if ch as u32 >= 0x80 && ch as u32 <= 0x9f => { + write!(self.inner, "\\u{{{:x}}}", ch as u32)? + }, + + _ => self.inner.write_char(ch)?, + } + } + Ok(()) + } +} + +impl fmt::Debug for Escape { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut escaping_writer = EscapingWriter { inner: f }; + write!(escaping_writer, "{:?}", self.0) + } +} + +impl fmt::Display for Escape { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut escaping_writer = EscapingWriter { inner: f }; + write!(escaping_writer, "{}", self.0) + } +} \ No newline at end of file diff --git a/tracing-subscriber/src/fmt/format/mod.rs b/tracing-subscriber/src/fmt/format/mod.rs index f5da2da66f..f72422bf85 100644 --- a/tracing-subscriber/src/fmt/format/mod.rs +++ b/tracing-subscriber/src/fmt/format/mod.rs @@ -48,6 +48,10 @@ use tracing_log::NormalizeEvent; #[cfg(feature = "ansi")] use nu_ansi_term::{Color, Style}; + +mod escape; +use escape::Escape; + #[cfg(feature = "json")] mod json; #[cfg(feature = "json")] @@ -1257,7 +1261,7 @@ impl field::Visit for DefaultVisitor<'_> { field, &format_args!( "{} {}{}{}{}", - value, + Escape(&format_args!("{}", value)), italic.paint(field.name()), italic.paint(".sources"), self.writer.dimmed().paint("="), @@ -1265,7 +1269,7 @@ impl field::Visit for DefaultVisitor<'_> { ), ) } else { - self.record_debug(field, &format_args!("{}", value)) + self.record_debug(field, &format_args!("{}", Escape(&format_args!("{}", value)))) } } @@ -1287,7 +1291,10 @@ impl field::Visit for DefaultVisitor<'_> { self.maybe_pad(); self.result = match name { - "message" => write!(self.writer, "{:?}", value), + "message" => { + // Escape ANSI characters to prevent malicious patterns (e.g., terminal injection attacks) + write!(self.writer, "{:?}", Escape(value)) + }, name if name.starts_with("r#") => write!( self.writer, "{}{}{:?}", @@ -1326,7 +1333,7 @@ impl Display for ErrorSourceList<'_> { let mut list = f.debug_list(); let mut curr = Some(self.0); while let Some(curr_err) = curr { - list.entry(&format_args!("{}", curr_err)); + list.entry(&Escape(&format_args!("{}", curr_err))); curr = curr_err.source(); } list.finish() diff --git a/tracing-subscriber/src/fmt/format/pretty.rs b/tracing-subscriber/src/fmt/format/pretty.rs index a6713542ae..9b89b6e60f 100644 --- a/tracing-subscriber/src/fmt/format/pretty.rs +++ b/tracing-subscriber/src/fmt/format/pretty.rs @@ -457,7 +457,7 @@ impl field::Visit for PrettyVisitor<'_> { field, &format_args!( "{}, {}{}.sources{}: {}", - value, + Escape(&format_args!("{}", value)), bold.prefix(), field, bold.infix(self.style), @@ -465,7 +465,7 @@ impl field::Visit for PrettyVisitor<'_> { ), ) } else { - self.record_debug(field, &format_args!("{}", value)) + self.record_debug(field, &Escape(&format_args!("{}", value))) } } @@ -475,7 +475,10 @@ impl field::Visit for PrettyVisitor<'_> { } let bold = self.bold(); match field.name() { - "message" => self.write_padded(&format_args!("{}{:?}", self.style.prefix(), value,)), + "message" => { + // Escape ANSI characters to prevent malicious patterns (e.g., terminal injection attacks) + self.write_padded(&format_args!("{}{:?}", self.style.prefix(), Escape(value))) + }, // Skip fields that are actually log metadata that have already been handled #[cfg(feature = "tracing-log")] name if name.starts_with("log.") => self.result = Ok(()), diff --git a/tracing-subscriber/tests/ansi_escaping.rs b/tracing-subscriber/tests/ansi_escaping.rs new file mode 100644 index 0000000000..120a44b588 --- /dev/null +++ b/tracing-subscriber/tests/ansi_escaping.rs @@ -0,0 +1,281 @@ +use std::sync::{Arc, Mutex}; +use tracing_subscriber::fmt::MakeWriter; + +/// Shared test writer that collects output for verification +#[derive(Debug, Clone)] +struct TestWriter { + buf: Arc>>, +} + +impl TestWriter { + fn new() -> Self { + Self { + buf: Arc::new(Mutex::new(Vec::new())), + } + } + + fn get_output(&self) -> String { + let buf = self.buf.lock().unwrap(); + String::from_utf8_lossy(&buf).to_string() + } +} + +impl std::io::Write for TestWriter { + fn write(&mut self, buf: &[u8]) -> std::io::Result { + self.buf.lock().unwrap().extend_from_slice(buf); + Ok(buf.len()) + } + + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } +} + +impl<'a> MakeWriter<'a> for TestWriter { + type Writer = TestWriter; + + fn make_writer(&'a self) -> Self::Writer { + self.clone() + } +} + +/// Test that basic security expectations are met - this is a smoke test +/// for the ANSI escaping functionality using public APIs only +#[test] +fn test_error_ansi_escaping() { + use std::fmt; + + #[derive(Debug)] + struct MaliciousError(&'static str); + + impl fmt::Display for MaliciousError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.0) + } + } + + impl std::error::Error for MaliciousError {} + + let writer = TestWriter::new(); + let subscriber = tracing_subscriber::fmt::Subscriber::builder() + .with_writer(writer.clone()) + .with_ansi(false) + .without_time() + .with_target(false) + .with_level(false) + .finish(); + + tracing::subscriber::with_default(subscriber, || { + let malicious_error = MaliciousError("\x1b]0;PWNED\x07\x1b[2J\x08\x0c\x7f"); + + // This demonstrates that errors are logged - the actual escaping + // is tested by our internal unit tests + tracing::error!(error = %malicious_error, "An error occurred"); + }); + + let output = writer.get_output(); + + // Just verify that something was logged + assert!( + output.contains("An error occurred"), + "Error message should be logged" + ); +} + +/// Test that ANSI escape sequences in log messages are properly escaped +#[test] +fn test_message_ansi_escaping() { + let writer = TestWriter::new(); + let subscriber = tracing_subscriber::fmt::Subscriber::builder() + .with_writer(writer.clone()) + .with_ansi(false) + .without_time() + .with_target(false) + .with_level(false) + .finish(); + + tracing::subscriber::with_default(subscriber, || { + let malicious_input = "\x1b]0;PWNED\x07\x1b[2J\x08\x0c\x7f"; + + // This should not cause ANSI injection + tracing::info!("User input: {}", malicious_input); + }); + + let output = writer.get_output(); + + // Verify ANSI sequences are escaped + assert!( + !output.contains('\x1b'), + "Message output should not contain raw ESC characters" + ); + assert!( + !output.contains('\x07'), + "Message output should not contain raw BEL characters" + ); +} + +/// Test that JSON formatter properly escapes ANSI sequences +#[cfg(feature = "json")] +#[test] +fn test_json_ansi_escaping() { + let writer = TestWriter::new(); + let subscriber = tracing_subscriber::fmt::Subscriber::builder() + .json() + .with_writer(writer.clone()) + .finish(); + + tracing::subscriber::with_default(subscriber, || { + let malicious_input = "\x1b]0;PWNED\x07\x1b[2J"; + + // JSON formatter should escape ANSI sequences + tracing::info!("Testing: {}", malicious_input); + tracing::info!(user_input = %malicious_input, "Field test"); + }); + + let output = writer.get_output(); + + // JSON should escape ANSI sequences as Unicode escapes + assert!( + !output.contains('\x1b'), + "JSON output should not contain raw ESC characters" + ); + assert!( + !output.contains('\x07'), + "JSON output should not contain raw BEL characters" + ); +} + +/// Test that pretty formatter properly escapes ANSI sequences +#[cfg(feature = "ansi")] +#[test] +fn test_pretty_ansi_escaping() { + let writer = TestWriter::new(); + let subscriber = tracing_subscriber::fmt::Subscriber::builder() + .pretty() + .with_writer(writer.clone()) + .with_ansi(false) + .without_time() + .with_target(false) + .finish(); + + tracing::subscriber::with_default(subscriber, || { + let malicious_input = "\x1b]0;PWNED\x07\x1b[2J"; + + // Pretty formatter should escape ANSI sequences + tracing::info!("Testing: {}", malicious_input); + }); + + let output = writer.get_output(); + + // Verify ANSI sequences are escaped + assert!( + !output.contains('\x1b'), + "Pretty output should not contain raw ESC characters" + ); + assert!( + !output.contains('\x07'), + "Pretty output should not contain raw BEL characters" + ); +} + +/// Comprehensive test for ANSI sanitization that prevents injection attacks +#[test] +fn ansi_sanitization_prevents_injection() { + let writer = TestWriter::new(); + let subscriber = tracing_subscriber::fmt::Subscriber::builder() + .with_writer(writer.clone()) + .with_ansi(false) + .without_time() + .with_target(false) + .with_level(false) + .finish(); + + #[derive(Debug)] + struct MaliciousError { + content: String, + } + + impl std::fmt::Display for MaliciousError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + // This Display implementation contains ANSI escape sequences + write!(f, "Error: {}", self.content) + } + } + + tracing::subscriber::with_default(subscriber, || { + // Test 1: Field values should remain properly escaped by Debug (baseline) + let malicious_field_value = "\x1b]0;PWNED\x07\x1b[2J"; + tracing::error!(malicious_field = malicious_field_value, "Field test"); + + // Test 2: Message content vulnerability should be mitigated + let malicious_error = MaliciousError { + content: "\x1b]0;PWNED\x07\x1b[2J".to_string(), + }; + tracing::error!("{}", malicious_error); + }); + + let output = writer.get_output(); + + // Field values should contain escaped sequences like \u{1b} + assert!( + output.contains("\\u{1b}"), + "Field values should be escaped by Debug formatting" + ); + + // Message content should be sanitized + assert!( + output.contains("\\x1b"), + "Message content should be sanitized" + ); + assert!( + !output.contains("\x1b]0;PWNED"), + "Message content should not contain raw ANSI sequences" + ); + assert!( + !output.contains("\x07"), + "Message content should not contain raw control characters" + ); +} + +/// Test that C1 control characters (\x80-\x9f) are also properly escaped +#[test] +fn test_c1_control_characters_escaping() { + let writer = TestWriter::new(); + let subscriber = tracing_subscriber::fmt::Subscriber::builder() + .with_writer(writer.clone()) + .with_ansi(false) + .without_time() + .with_target(false) + .with_level(false) + .finish(); + + tracing::subscriber::with_default(subscriber, || { + // Test C1 control characters that can be used in 8-bit terminal escape sequences + let c1_controls = "\u{80}\u{85}\u{90}\u{9b}\u{9c}\u{9d}\u{9e}\u{9f}"; // Various C1 controls including CSI + + // This should escape C1 control characters to prevent 8-bit escape sequences + tracing::info!("C1 controls: {}", c1_controls); + }); + + let output = writer.get_output(); + + // Verify C1 control characters are escaped + assert!( + !output.contains('\u{80}'), + "Output should not contain raw C1 control characters" + ); + assert!( + !output.contains('\u{9b}'), + "Output should not contain raw CSI character" + ); + assert!( + !output.contains('\u{9c}'), + "Output should not contain raw ST character" + ); + + // Should contain Unicode escapes for C1 characters + assert!( + output.contains("\\u{80}") || output.contains("\\u{8"), + "Should contain escaped C1 characters" + ); +}