From f3dcf46a85c2f623bafe56d21f96feff447abb86 Mon Sep 17 00:00:00 2001 From: Wez Furlong Date: Sat, 26 Sep 2026 22:28:46 +0100 Subject: [PATCH] dkim: Fix remote DoS from crafted DKIM signatures during verification This fixes two vulnerabilities reachable via msg:dkim_verify() on inbound mail: an out-of-bounds read panic on very short b= tags, and the anti-DoS cap now correctly bounds all parsed signatures, not just parse failures. --- crates/dkim/src/lib.rs | 114 +++++++++++++++++++++++++++++++++++------ docs/changelog/main.md | 8 +++ 2 files changed, 106 insertions(+), 16 deletions(-) diff --git a/crates/dkim/src/lib.rs b/crates/dkim/src/lib.rs index a697fc767..24c2ead5c 100644 --- a/crates/dkim/src/lib.rs +++ b/crates/dkim/src/lib.rs @@ -290,9 +290,10 @@ pub async fn verify_email_with_resolver<'a>( let mut dkim_headers = vec![]; for h in email.get_headers().iter_named(DKIM_SIGNATURE_HEADER_NAME) { - if results.len() > 10 { - // Limit DoS impact if a malicious message is filled - // with signatures + // Limit DoS impact if a malicious message is filled with signatures. + // Both failed parses (results) and successful ones (dkim_headers, each + // of which drives a DNS lookup and a verification) count against the cap. + if results.len() + dkim_headers.len() > 10 { break; } @@ -323,22 +324,27 @@ pub async fn verify_email_with_resolver<'a>( /// relayed, and MUST be long enough to be unique among the results being /// reported. fn compute_header_b(b_tag: &str, headers: &[DKIMHeader]) -> String { - let mut len = 8; + let total = b_tag.chars().count(); - 'bigger: while len < b_tag.len() { - for h in headers { - let candidate = h.get_required_tag("b"); - if candidate == b_tag { - continue; - } - if b_tag[0..len] == candidate[0..len] { - len += 2; - continue 'bigger; - } + // At least the first eight characters, and never more than the whole tag. + let mut needed = total.min(8); + + for h in headers { + let candidate = h.get_required_tag("b"); + if candidate == b_tag { + continue; } - return b_tag[0..len].to_string(); + // One character past the shared leading run distinguishes b_tag from + // this candidate; extend to cover it, up to the whole tag. + let shared = b_tag + .chars() + .zip(candidate.chars()) + .take_while(|(a, b)| a == b) + .count(); + needed = needed.max((shared + 1).min(total)); } - b_tag.to_string() + + b_tag.chars().take(needed).collect() } for dkim_header in &dkim_headers { @@ -480,6 +486,82 @@ b=dzdVyOfAKCdLXdJOc9G2q8LoXSlEniSbav+yuU4zGeeruD00lszZ ); } + #[tokio::test] + async fn test_short_b_tag_does_not_panic() { + // Two signatures whose b= tags don't share a common prefix, where one + // b= is shorter than the 8 character minimum that compute_header_b + // starts from. The shorter value must not be sliced past its length. + let raw_email = concat!( + "DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=example.com; s=sel1; h=From; bh=AAAA; b=short\r\n", + "DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=example.com; s=sel2; h=From; bh=AAAA; b=alongerbase64valuethatexceedseightbytes\r\n", + "From: user@example.com\r\n\r\nhello", + ); + + let email = ParsedEmail::parse(raw_email).unwrap(); + let resolver = TestResolver::default(); + + let res = verify_email_with_resolver(&email, &resolver).await.unwrap(); + assert_eq!(res.len(), 2); + + // The short tag is reported whole. The long tag is trimmed to the eight + // character minimum since it doesn't share a prefix with the short one. + assert_eq!( + res[0].props.get("header.b").unwrap().to_str().unwrap(), + "short" + ); + assert_eq!( + res[1].props.get("header.b").unwrap().to_str().unwrap(), + "alongerb" + ); + } + + async fn header_b_values(raw_email: &str) -> Vec { + let email = ParsedEmail::parse(raw_email).unwrap(); + let resolver = TestResolver::default(); + verify_email_with_resolver(&email, &resolver) + .await + .unwrap() + .iter() + .map(|r| { + r.props + .get("header.b") + .unwrap() + .to_str() + .unwrap() + .to_string() + }) + .collect() + } + + #[tokio::test] + async fn test_header_b_prefix_extension() { + // Two b= tags sharing exactly the eight character floor must each + // extend to nine to stay distinct, proving the floor is not a ceiling. + assert_eq!( + header_b_values(concat!( + "DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=example.com; s=s1; h=From; bh=AAAA; b=AAAAAAAAX\r\n", + "DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=example.com; s=s2; h=From; bh=AAAA; b=AAAAAAAAY\r\n", + "From: user@example.com\r\n\r\nhello", + )) + .await, + vec!["AAAAAAAAX", "AAAAAAAAY"] + ); + + // A long shared prefix drives the length past the floor. The length is + // the max across candidates (not the nearest one), capped at the tag + // itself when a tag is a strict prefix of another. + assert_eq!( + header_b_values(concat!( + "DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=example.com; s=s1; h=From; bh=AAAA; b=COMMONPREFIXAAAAAA\r\n", + "DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=example.com; s=s2; h=From; bh=AAAA; b=COMMONPREFIXAB\r\n", + "DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=example.com; s=s3; h=From; bh=AAAA; b=COMMONXYZ\r\n", + "From: user@example.com\r\n\r\nhello", + )) + .await, + vec!["COMMONPREFIXAA", "COMMONPREFIXAB", "COMMONXY"] + ); + } + #[tokio::test] async fn test_expired_signature_populates_props() { let x_value = "1"; diff --git a/docs/changelog/main.md b/docs/changelog/main.md index 7d1dd6636..d78634a0f 100644 --- a/docs/changelog/main.md +++ b/docs/changelog/main.md @@ -33,6 +33,14 @@ `max_line_size` (default 128 MiB). A record larger than that limit is dropped, and the rest of the segment is still read. + * Fixed a remotely triggerable panic in DKIM verification. A message with two + or more `DKIM-Signature` headers whose `b=` tags differed in length, one of + them shorter than eight characters, panicked with an out-of-bounds read while + computing the `header.b` authentication result. This was reachable on inbound + mail through `msg:dkim_verify()`. The signature-count limit that caps + verification work per message now also counts successfully parsed + signatures, not only signatures that failed to parse. + * Fixed an integer underflow when re-encoding a MIME parameter (such as a `Content-Type` parameter) whose name was long enough that the fold framing exceeded the target line width. This panicked in debug builds and produced