From da20ce60e11d0f99258f0f0da47d102866c4f3e6 Mon Sep 17 00:00:00 2001 From: ygd58 Date: Tue, 21 Jul 2026 12:38:27 +0000 Subject: [PATCH] fix(email): don't let quoted display-name brackets override the real sender address extract_sender_address() picked the address from the *last* <...> pair found anywhere in the From header. If a quoted display-name that follows the real address happens to contain its own bracketed text, e.g.: "Bob " the function returned the decoy fake@evil.com instead of the real sender alice@trusted.org, because rfind('<')/rfind('>') don't know about quoting. Since EmailPolicy::sender_visible() drives allowlist/denylist filtering directly off this value, a crafted display-name could cause a message to be mis-classified relative to the actual sender. Replaced the naive last-bracket search with extract_bracketed_address(), a single left-to-right scan that tracks whether it's inside a double-quoted quoted-string (respecting backslash-escaped quotes) and only treats unquoted '<'/'>' as address delimiters. This correctly resolves the real address regardless of whether a bracketed decoy appears before or after it in the header. Added regression tests for: decoy after the real address, decoy before it (previously-working direction, kept as a guard), and an escaped-quote edge case inside the display-name. Note on verification: this sandbox only has rustc 1.75 available via apt, and the crate needs edition 2024 (~1.85+), so I couldn't run the full 'cargo test' suite here. I extracted the new function verbatim into a standalone file and compiled+ran it with rustc directly against all 5 test cases (3 old passing behaviors + 2 new ones) -- all passed. Please run 'cargo test --lib email' locally to confirm before merging. --- src/email.rs | 88 ++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 79 insertions(+), 9 deletions(-) diff --git a/src/email.rs b/src/email.rs index 1427510..75587fe 100644 --- a/src/email.rs +++ b/src/email.rs @@ -997,15 +997,8 @@ pub fn extract_sender_address(from_header: &str) -> Option { return None; } - if let Some((start, end)) = from_header - .rfind('<') - .zip(from_header.rfind('>')) - .filter(|(start, end)| start < end) - { - let candidate = from_header[start + 1..end].trim(); - if candidate.contains('@') { - return Some(candidate.to_ascii_lowercase()); - } + if let Some(candidate) = extract_bracketed_address(from_header) { + return Some(candidate); } from_header.split_whitespace().find_map(|token| { @@ -1018,6 +1011,53 @@ pub fn extract_sender_address(from_header: &str) -> Option { }) } +/// Locates the `` portion of a From header per RFC 5322 +/// (`mailbox = [display-name] "<" addr-spec ">"`), ignoring any `<`/`>` +/// characters that appear inside a quoted display-name. +/// +/// A naive "take the last `<...>` pair in the header" search can be fooled +/// by a display-name that itself contains bracketed text, e.g.: +/// +/// ` "Bob "` +/// +/// Here the real sender is `alice@trusted.org`, but a last-bracket search +/// returns the decoy `fake@evil.com` from inside the quoted string, which +/// would let a crafted display-name bypass sender allowlist/denylist +/// filtering. This scans left-to-right, tracks whether it is inside a +/// double-quoted quoted-string (respecting backslash-escapes), and only +/// treats unquoted `<`/`>` as address delimiters. +fn extract_bracketed_address(from_header: &str) -> Option { + let mut in_quotes = false; + let mut start = None; + let mut chars = from_header.char_indices().peekable(); + + while let Some((i, ch)) = chars.next() { + match ch { + '\\' if in_quotes => { + // Skip an escaped character inside a quoted-string so an + // escaped quote (`\"`) doesn't flip `in_quotes`. + chars.next(); + } + '"' => in_quotes = !in_quotes, + '<' if !in_quotes && start.is_none() => start = Some(i), + '>' if !in_quotes => { + if let Some(s) = start { + let candidate = from_header[s + 1..i].trim(); + if candidate.contains('@') { + return Some(candidate.to_ascii_lowercase()); + } + // Not a real address (empty, or no '@') — keep scanning + // in case another unquoted bracket pair follows. + start = None; + } + } + _ => {} + } + } + + None +} + pub fn sender_matches_rule(sender_email: &str, rule: &str) -> bool { let sender_email = sender_email.to_ascii_lowercase(); let rule = normalize_sender_rule(rule); @@ -1047,6 +1087,36 @@ mod tests { assert_eq!(sender, "user@example.com"); } + #[test] + fn test_extract_sender_address_ignores_bracket_in_quoted_display_name_after() { + // Real address comes first; a quoted display-name *after* it + // contains a decoy bracketed address. A naive "last bracket pair" + // search would wrongly return the decoy. + let sender = + extract_sender_address(r#" "Bob ""#).unwrap(); + assert_eq!(sender, "alice@trusted.org"); + } + + #[test] + fn test_extract_sender_address_ignores_bracket_in_quoted_display_name_before() { + // Decoy comes first inside the quoted display-name, real address + // follows — this direction already worked before the fix, kept as + // a regression guard. + let sender = + extract_sender_address(r#""Bob " "#).unwrap(); + assert_eq!(sender, "alice@trusted.org"); + } + + #[test] + fn test_extract_sender_address_handles_escaped_quote_in_display_name() { + // An escaped quote inside the display-name shouldn't flip the + // "in a quoted-string" state early and expose the decoy bracket. + let sender = + extract_sender_address(r#""Bob \"the < sign\" " "#) + .unwrap(); + assert_eq!(sender, "alice@trusted.org"); + } + #[test] fn test_sender_matches_rule_exact() { assert!(sender_matches_rule(