From a0ff0d71bc62445af4130889c8aacec27f9e57f8 Mon Sep 17 00:00:00 2001 From: link2xt Date: Fri, 7 Feb 2025 04:11:42 +0000 Subject: [PATCH] fix: do not include CRLF before MIME boundary in the part body This change adds a test and updates mailparse from 0.15.0 to 0.16.0. mailparse 0.16.0 includes a fix for the bug that resulted in CRLF being included at the end of the body. Workaround for the bug in the `pk_validate` function is also removed. --- Cargo.lock | 4 +- Cargo.toml | 2 +- src/html.rs | 3 -- src/mimeparser/mimeparser_tests.rs | 75 ++++++++++++++++++++++++++++++ src/pgp.rs | 7 --- 5 files changed, 78 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4cb4473867..a6635634c2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3596,9 +3596,9 @@ checksum = "9106e1d747ffd48e6be5bb2d97fa706ed25b144fbee4d5c02eae110cd8d6badd" [[package]] name = "mailparse" -version = "0.15.0" +version = "0.16.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3da03d5980411a724e8aaf7b61a7b5e386ec55a7fb49ee3d0ff79efc7e5e7c7e" +checksum = "cdd4a624f609f8ea7366324724b7ff41cb0f982eb665a54ac825bc1c0efc07a3" dependencies = [ "charset", "data-encoding", diff --git a/Cargo.toml b/Cargo.toml index a89efb1f05..e1efc2e643 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -69,7 +69,7 @@ iroh = { version = "0.32", default-features = false } kamadak-exif = "0.6.1" lettre_email = { git = "https://github.com/deltachat/lettre", branch = "master" } libc = { workspace = true } -mailparse = "0.15" +mailparse = "0.16" mime = "0.3.17" num_cpus = "1.16" num-derive = "0.4" diff --git a/src/html.rs b/src/html.rs index 1e5e5097c9..7c26ffb475 100644 --- a/src/html.rs +++ b/src/html.rs @@ -371,7 +371,6 @@ and will be wrapped as usual.
mime-modified should not be set set as there is no html and no special stuff;
although not being a delta-message.
test some special html-characters as < > and & but also " and ' :)
-
"# ); @@ -405,7 +404,6 @@ test some special html-characters as < > and & but also " and &#x r##"

mime-modified set; simplify is always regarded as lossy.

- "## ); } @@ -422,7 +420,6 @@ test some special html-characters as < > and & but also " and &#x this is html

- "## ); } diff --git a/src/mimeparser/mimeparser_tests.rs b/src/mimeparser/mimeparser_tests.rs index ac738545b1..4d48e25a8d 100644 --- a/src/mimeparser/mimeparser_tests.rs +++ b/src/mimeparser/mimeparser_tests.rs @@ -1817,3 +1817,78 @@ async fn test_protect_autocrypt() -> Result<()> { Ok(()) } + +/// Tests that CRLF before MIME boundary +/// is not treated as the part body. +/// +/// RFC 2046 explicitly says that +/// "The CRLF preceding the boundary delimiter line is conceptually attached +/// to the boundary so that it is possible to have a part that does not end +/// with a CRLF (line break). Body parts that must be considered to end with +/// line breaks, therefore, must have two CRLFs preceding the boundary delimiter +/// line, the first of which is part of the preceding body part, +/// and the second of which is part of the encapsulation boundary." +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_mimeparser_trailing_newlines() { + let context = TestContext::new_alice().await; + + // Example from + // with a `Content-Disposition` headers added to turn files + // into attachments. + let raw = b"From: Nathaniel Borenstein \r +To: Ned Freed \r +Date: Sun, 21 Mar 1993 23:56:48 -0800 (PST)\r +Subject: Sample message\r +MIME-Version: 1.0\r +Content-type: multipart/mixed; boundary=\"simple boundary\"\r +\r +This is the preamble. It is to be ignored, though it\r +is a handy place for composition agents to include an\r +explanatory note to non-MIME conformant readers.\r +\r +--simple boundary\r +Content-Disposition: attachment; filename=\"file1.txt\"\r +\r +This is implicitly typed plain US-ASCII text.\r +It does NOT end with a linebreak.\r +--simple boundary\r +Content-type: text/plain; charset=us-ascii\r +Content-Disposition: attachment; filename=\"file2.txt\"\r +\r +This is explicitly typed plain US-ASCII text.\r +It DOES end with a linebreak.\r +\r +--simple boundary--\r +\r +This is the epilogue. It is also to be ignored."; + + let mimeparser = MimeMessage::from_bytes(&context, &raw[..], None) + .await + .unwrap(); + + assert_eq!(mimeparser.parts.len(), 2); + + assert_eq!(mimeparser.parts[0].typ, Viewtype::File); + let blob: BlobObject = mimeparser.parts[0] + .param + .get_blob(Param::File, &context) + .await + .unwrap() + .unwrap(); + assert_eq!( + tokio::fs::read_to_string(blob.to_abs_path()).await.unwrap(), + "This is implicitly typed plain US-ASCII text.\r\nIt does NOT end with a linebreak." + ); + + assert_eq!(mimeparser.parts[1].typ, Viewtype::File); + let blob: BlobObject = mimeparser.parts[1] + .param + .get_blob(Param::File, &context) + .await + .unwrap() + .unwrap(); + assert_eq!( + tokio::fs::read_to_string(blob.to_abs_path()).await.unwrap(), + "This is explicitly typed plain US-ASCII text.\r\nIt DOES end with a linebreak.\r\n" + ); +} diff --git a/src/pgp.rs b/src/pgp.rs index 804d520c09..48db25bc60 100644 --- a/src/pgp.rs +++ b/src/pgp.rs @@ -379,13 +379,6 @@ pub fn pk_validate( let standalone_signature = StandaloneSignature::from_armor_single(Cursor::new(signature))?.0; - // Remove trailing CRLF before the delimiter. - // According to RFC 3156 it is considered to be part of the MIME delimiter for the purpose of - // OpenPGP signature calculation. - let content = content - .get(..content.len().saturating_sub(2)) - .context("index is out of range")?; - for pkey in public_keys_for_validation { if standalone_signature.verify(pkey, content).is_ok() { let fp = pkey.dc_fingerprint();