From a65e7a14902ea6a7579b1bafb8b665f533793f6e Mon Sep 17 00:00:00 2001 From: l Date: Tue, 12 May 2026 05:00:21 +0000 Subject: [PATCH] fix: Validate mail data (#150) Do not allow bare CR and LF etc. --- filtermail/Cargo.lock | 5 +- filtermail/Cargo.toml | 1 + filtermail/src/smtp_server.rs | 88 +++++++++++++++++++++++++++++++++++ 3 files changed, 92 insertions(+), 2 deletions(-) diff --git a/filtermail/Cargo.lock b/filtermail/Cargo.lock index bb682515..0356122e 100644 --- a/filtermail/Cargo.lock +++ b/filtermail/Cargo.lock @@ -522,6 +522,7 @@ dependencies = [ "log", "lru", "mailparse", + "memchr", "parking_lot", "retainer", "rstest", @@ -1312,9 +1313,9 @@ dependencies = [ [[package]] name = "memchr" -version = "2.7.6" +version = "2.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f52b00d39961fc5b2736ea853c9cc86238e165017a493d1d5c8eac6bdc4cc273" +checksum = "f8ca58f447f06ed17d5fc4043ce1b10dd205e060fb3ce5b979b8ed8e59ff3f79" [[package]] name = "mio" diff --git a/filtermail/Cargo.toml b/filtermail/Cargo.toml index a07f00c2..a5b1b74c 100644 --- a/filtermail/Cargo.toml +++ b/filtermail/Cargo.toml @@ -16,6 +16,7 @@ serini = "0.2.2" tokio = { version = "1.52.1", features = ["full"] } thiserror = "2.0.18" mailparse = "0.16.1" +memchr = "2.8.0" log = "0.4.29" env_logger = "0.11.10" governor = "0.10.4" diff --git a/filtermail/src/smtp_server.rs b/filtermail/src/smtp_server.rs index 62aef491..818d9eb3 100644 --- a/filtermail/src/smtp_server.rs +++ b/filtermail/src/smtp_server.rs @@ -2,6 +2,7 @@ use crate::utils::{extract_address, log_eml}; use async_trait::async_trait; +use memchr::{Memchr, memmem}; use std::sync::Arc; use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader, BufWriter}; use tokio::net::{TcpListener, TcpStream}; @@ -12,9 +13,48 @@ pub struct Envelope { pub mail_from: String, pub origin_ip: String, pub rcpt_to: Vec, + + /// Mail data as transmitted over SMTP/LMTP. + /// + /// Described in . + /// + /// It MUST end with ``, contain no bare `` or `` + /// and have all `.` sequences escaped with `.` according to + /// . pub data: Vec, } +/// Checks if mail data is valid. +fn is_valid_data(data: &[u8]) -> bool { + // DATA must end with . + // + // Otherwise it is not possible to reinject it as is into SMTP/LMTP + // without adding at the end and modifying the message. + if !data.ends_with(b"\r\n") { + return false; + } + + // Check for bare `` and ``. + // + for pos in Memchr::new(b'\r', data) { + if data.get(pos + 1) != Some(&b'\n') { + return false; + } + } + for pos in Memchr::new(b'\n', data) { + if pos == 0 || data.get(pos - 1) != Some(&b'\r') { + return false; + } + } + + // Do not allow unescaped `.`. + if data.starts_with(b".\r\n") || memmem::find(data, b"\r\n.\r\n").is_some() { + return false; + } + + true +} + /// Trait defining the SMTP handler interface. #[async_trait] pub trait SmtpHandler: Send + Sync { @@ -32,6 +72,17 @@ pub trait SmtpHandler: Send + Sync { /// Handles the DATA command. async fn handle_data(&self, envelope: &mut Envelope) -> Result { log::debug!("handle_DATA before-queue"); + + // Check if the DATA is valid + // before doing any custom checks. + // + // We are not going to normalize newlines + // and escape the dots in the mail data. + // If mail data turned out to be invalid, reject immediately. + if !is_valid_data(&envelope.data) { + return Err("500 Invalid DATA".to_string()); + } + self.check_data(envelope).await?; if envelope.rcpt_to.is_empty() { log::warn!("Dropping mail; All recipients disabled."); @@ -246,3 +297,40 @@ where Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + use rstest::*; + + #[rstest] + #[case(b"", false)] + #[case(b".", false)] + #[case(b"Hello!\n", false)] + #[case(b"Hello!\n\r", false)] + #[case(b"Hello\nworld!\r\n", false)] + #[case(b"Hello!\r\n\n", false)] + #[case(b"Hello\r\n.\r\n", false)] + #[case(b"Hello!\r\n .\r\n", true)] + #[case(b"Hello!\r\n..\r\n", true)] + #[case(b"Hello!\r\n", true)] + #[case(b"Hello\r\n.world\r\n", true)] + #[case(b"Hello!\r\r\n", false)] + #[case(b"Hello!\r\r\n\n", false)] + #[case(b"Hello\rworld!\r\n", false)] + #[case(b"\n", false)] + #[case(b"\nHello\r\n", false)] + #[case(b"\r", false)] + #[case(b".\r\n", false)] + #[case(b".\r\nHello\r\n", false)] + #[case(b"..\r\n.\r\n", false)] + #[case(b".\r\n..\r\n", false)] + #[case(b"\r\n.\r\n", false)] + #[case(b"..\r\n..\r\n", true)] + #[case(b" .\r\n", true)] + #[case(b"..\r\n", true)] + #[case(b"\r\n", true)] + fn test_is_valid_data(#[case] data: &[u8], #[case] expected: bool) { + assert_eq!(is_valid_data(data), expected, "{data:?}"); + } +}