diff --git a/tracing-subscriber/src/fmt/format/escape.rs b/tracing-subscriber/src/fmt/format/escape.rs index 00837b04b3..3d78849766 100644 --- a/tracing-subscriber/src/fmt/format/escape.rs +++ b/tracing-subscriber/src/fmt/format/escape.rs @@ -2,50 +2,68 @@ use std::fmt::{self, Write}; -/// A wrapper that implements `fmt::Debug` and `fmt::Display` and escapes ANSI sequences on-the-fly. +/// A wrapper that implements `fmt::Debug` and escapes control sequences on-the-fly. /// This avoids creating intermediate strings while providing security against terminal injection. pub(super) struct Escape(pub(super) T); +impl fmt::Debug for Escape { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut escaping_writer = EscapingWriter { + inner: f, + skip_csi_codes: false, + }; + write!(escaping_writer, "{:?}", self.0) + } +} + +/// A wrapper that implements `fmt::Debug` and removes control sequences on-the-fly. +/// This avoids creating intermediate strings while providing security against terminal injection. +pub(super) struct EscapeSkip(pub(super) T); + +impl fmt::Debug for EscapeSkip { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut escaping_writer = EscapingWriter { + inner: f, + skip_csi_codes: true, + }; + write!(escaping_writer, "{:?}", self.0) + } +} + /// Helper struct that escapes ANSI sequences as characters are written struct EscapingWriter<'a, 'b> { inner: &'a mut fmt::Formatter<'b>, + skip_csi_codes: bool, } 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 => { + // Stream the string character by character, escaping all control sequences + let mut chars = s.chars().peekable(); + while let Some(ch) = chars.next() { + // Recognize and remove ECMA-48 CSI codes + // This is a best effort to clean up colour codes in the message field + if ch == '\x1b' && self.skip_csi_codes { + if chars.next_if_eq(&'[').is_some() { + // Remove parameter and intermediate bytes + while chars.next_if(|x| matches!(x, '\x20'..='\x3f')).is_some() {} + // Remove final byte + chars.next_if(|x| matches!(x, '\x40'..='\x7E')); + continue; + } + } + + // ESC BEL BS FF DEL + if matches!(ch, '\x1b' | '\x07' | '\x08' | '\x0c' | '\x7f'..='\u{9f}') { + if ch.is_ascii() { + write!(self.inner, "\\x{:02x}", ch as u32)? + } else { write!(self.inner, "\\u{{{:x}}}", ch as u32)? - }, - - _ => self.inner.write_char(ch)?, + } + } else { + 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 bb7ff90df8..cb3c90b952 100644 --- a/tracing-subscriber/src/fmt/format/mod.rs +++ b/tracing-subscriber/src/fmt/format/mod.rs @@ -400,7 +400,7 @@ pub struct Full; /// span context, but other information is abbreviated. The [`Pretty`] logging /// format is an extra-verbose, multi-line human-readable logging format /// intended for use in development. -/// +/// /// [`FmtSubscriber`]: super::Subscriber #[derive(Debug, Clone)] pub struct Format { @@ -1262,7 +1262,7 @@ impl field::Visit for DefaultVisitor<'_> { self.record_debug( field, &format_args!( - "{} {}{}{}{}", + "{:?} {}{}{}{}", Escape(&format_args!("{}", value)), italic.paint(field.name()), italic.paint(".sources"), @@ -1271,10 +1271,7 @@ impl field::Visit for DefaultVisitor<'_> { ), ) } else { - self.record_debug( - field, - &format_args!("{}", Escape(&format_args!("{}", value))), - ) + self.record_debug(field, &Escape(&format_args!("{}", value))) } } @@ -1601,7 +1598,7 @@ impl fmt::Debug for FieldFnVisitor<'_, F> { /// Configures what points in the span lifecycle are logged as events. /// /// See also [`with_span_events`]. -/// +/// /// [`with_span_events`]: super::SubscriberBuilder::with_span_events #[derive(Clone, Eq, PartialEq, Ord, PartialOrd)] pub struct FmtSpan(u8); diff --git a/tracing-subscriber/src/fmt/format/pretty.rs b/tracing-subscriber/src/fmt/format/pretty.rs index 9b89b6e60f..c4637c86ab 100644 --- a/tracing-subscriber/src/fmt/format/pretty.rs +++ b/tracing-subscriber/src/fmt/format/pretty.rs @@ -1,7 +1,10 @@ use super::*; use crate::{ field::{VisitFmt, VisitOutput}, - fmt::fmt_layer::{FmtContext, FormattedFields}, + fmt::{ + fmt_layer::{FmtContext, FormattedFields}, + format::escape::EscapeSkip, + }, registry::LookupSpan, }; @@ -419,6 +422,9 @@ impl<'a> PrettyVisitor<'a> { } fn write_padded(&mut self, value: &impl fmt::Debug) { + if self.result.is_err() { + return; + } let padding = if self.is_empty { self.is_empty = false; "" @@ -435,14 +441,24 @@ impl<'a> PrettyVisitor<'a> { Style::new() } } + + fn write_field(&mut self, mut name: &str, value: &dyn fmt::Debug) { + if name.starts_with("r#") { + name = &name[2..]; + } + let bold = self.bold(); + self.write_padded(&format_args!( + "{}{}{}: {:?}", + bold.prefix(), + name, + bold.infix(self.style), + Escape(value) + )) + } } impl field::Visit for PrettyVisitor<'_> { fn record_str(&mut self, field: &Field, value: &str) { - if self.result.is_err() { - return; - } - if field.name() == "message" { self.record_debug(field, &format_args!("{}", value)) } else { @@ -451,51 +467,30 @@ impl field::Visit for PrettyVisitor<'_> { } fn record_error(&mut self, field: &Field, value: &(dyn std::error::Error + 'static)) { + self.record_debug(field, &format_args!("{}", value)); + if let Some(source) = value.source() { - let bold = self.bold(); - self.record_debug( - field, - &format_args!( - "{}, {}{}.sources{}: {}", - Escape(&format_args!("{}", value)), - bold.prefix(), - field, - bold.infix(self.style), - ErrorSourceList(source), - ), - ) - } else { - self.record_debug(field, &Escape(&format_args!("{}", value))) + self.write_field( + &std::format!("{field}.sources"), + &format_args!("{}", ErrorSourceList(source)), + ); } } fn record_debug(&mut self, field: &Field, value: &dyn fmt::Debug) { - if self.result.is_err() { - return; - } - let bold = self.bold(); match field.name() { "message" => { // Escape ANSI characters to prevent malicious patterns (e.g., terminal injection attacks) - self.write_padded(&format_args!("{}{:?}", self.style.prefix(), Escape(value))) - }, + self.write_padded(&format_args!( + "{}{:?}", + self.style.prefix(), + EscapeSkip(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(()), - name if name.starts_with("r#") => self.write_padded(&format_args!( - "{}{}{}: {:?}", - bold.prefix(), - &name[2..], - bold.infix(self.style), - value - )), - name => self.write_padded(&format_args!( - "{}{}{}: {:?}", - bold.prefix(), - name, - bold.infix(self.style), - value - )), + name => self.write_field(name, value), }; } } diff --git a/tracing-subscriber/tests/ansi_escaping.rs b/tracing-subscriber/tests/ansi_escaping.rs index 120a44b588..fde26f19cd 100644 --- a/tracing-subscriber/tests/ansi_escaping.rs +++ b/tracing-subscriber/tests/ansi_escaping.rs @@ -145,7 +145,7 @@ fn test_json_ansi_escaping() { ); } -/// Test that pretty formatter properly escapes ANSI sequences +/// Test that pretty formatter properly escapes ANSI sequences #[cfg(feature = "ansi")] #[test] fn test_pretty_ansi_escaping() { @@ -160,9 +160,12 @@ fn test_pretty_ansi_escaping() { tracing::subscriber::with_default(subscriber, || { let malicious_input = "\x1b]0;PWNED\x07\x1b[2J"; + let colourful = "[\u{1b}[1m\u{1b}[38;5;9merror\u{1b}[0m\u{1b}[1m: Invalid JSON\u{1b}[0m]"; // Pretty formatter should escape ANSI sequences tracing::info!("Testing: {}", malicious_input); + tracing::info!(user_input = %malicious_input, "Field test"); + tracing::info!("Colour test: {}", colourful); }); let output = writer.get_output(); @@ -176,6 +179,10 @@ fn test_pretty_ansi_escaping() { !output.contains('\x07'), "Pretty output should not contain raw BEL characters" ); + assert!( + output.contains("[error: Invalid JSON]"), + "ansi escape code should be removed from message entirely" + ) } /// Comprehensive test for ANSI sanitization that prevents injection attacks