From 04fd2cdcab6b0a9486cca2e64f2196bc2d209b80 Mon Sep 17 00:00:00 2001
From: iequidoo <dgreshilov@gmail.com>
Date: Mon, 22 Jul 2024 13:33:05 -0300
Subject: [PATCH] fix: Reject message with forged From even if no valid
 signatures are found

There are many reasons why we may fail to find valid signatures in a message, e.g. we don't yet know
a public key attached in the same message, anyway, if From is forged, the message must be rejected.

Also always take the displayname from encrypted From, even if no valid signatures are found.
---
 src/mimeparser.rs        | 15 +++++++--------
 src/receive_imf/tests.rs | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 8 deletions(-)

diff --git a/src/mimeparser.rs b/src/mimeparser.rs
index 081b3b4e5..b4d740c01 100644
--- a/src/mimeparser.rs
+++ b/src/mimeparser.rs
@@ -396,13 +396,10 @@ impl MimeMessage {
                 &mail.headers,
             );
 
-            if let (Some(inner_from), true) = (inner_from, !signatures.is_empty()) {
-                if addr_cmp(&inner_from.addr, &from.addr) {
-                    from_is_signed = true;
-                    from = inner_from;
-                } else {
-                    // There is a From: header in the encrypted &
-                    // signed part, but it doesn't match the outer one.
+            if let Some(inner_from) = inner_from {
+                if !addr_cmp(&inner_from.addr, &from.addr) {
+                    // There is a From: header in the encrypted
+                    // part, but it doesn't match the outer one.
                     // This _might_ be because the sender's mail server
                     // replaced the sending address, e.g. in a mailing list.
                     // Or it's because someone is doing some replay attack.
@@ -411,7 +408,7 @@ impl MimeMessage {
                     // so we return an error below.
                     warn!(
                         context,
-                        "From header in signed part doesn't match the outer one",
+                        "From header in encrypted part doesn't match the outer one",
                     );
 
                     // Return an error from the parser.
@@ -420,6 +417,8 @@ impl MimeMessage {
                     // as if the MIME structure is broken.
                     bail!("From header is forged");
                 }
+                from = inner_from;
+                from_is_signed = !signatures.is_empty();
             }
         }
         if signatures.is_empty() {
diff --git a/src/receive_imf/tests.rs b/src/receive_imf/tests.rs
index 3dcfe9418..68181be2c 100644
--- a/src/receive_imf/tests.rs
+++ b/src/receive_imf/tests.rs
@@ -3559,6 +3559,39 @@ async fn test_prefer_encrypt_mutual_if_encrypted() -> Result<()> {
     Ok(())
 }
 
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn test_forged_from_and_no_valid_signatures() -> Result<()> {
+    let t = &TestContext::new_bob().await;
+    let raw = include_bytes!("../../test-data/message/thunderbird_encrypted_signed.eml");
+    let received_msg = receive_imf(t, raw, false).await?.unwrap();
+    assert!(!received_msg.from_is_signed);
+    let msg = t.get_last_msg().await;
+    assert!(!msg.chat_id.is_trash());
+    assert!(!msg.get_showpadlock());
+
+    let t = &TestContext::new_bob().await;
+    let raw = String::from_utf8(raw.to_vec())?.replace("alice@example.org", "clarice@example.org");
+    let received_msg = receive_imf(t, raw.as_bytes(), false).await?.unwrap();
+    assert!(received_msg.chat_id.is_trash());
+
+    Ok(())
+}
+
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn test_wrong_from_name_and_no_valid_signatures() -> Result<()> {
+    let t = &TestContext::new_bob().await;
+    let raw = include_bytes!("../../test-data/message/thunderbird_encrypted_signed.eml");
+    let raw = String::from_utf8(raw.to_vec())?.replace("From: Alice", "From: A");
+    let received_msg = receive_imf(t, raw.as_bytes(), false).await?.unwrap();
+    assert!(!received_msg.from_is_signed);
+    let msg = t.get_last_msg().await;
+    assert!(!msg.chat_id.is_trash());
+    assert!(!msg.get_showpadlock());
+    let contact = Contact::get_by_id(t, msg.from_id).await?;
+    assert_eq!(contact.get_authname(), "Alice");
+    Ok(())
+}
+
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
 async fn test_thunderbird_autocrypt_unencrypted() -> Result<()> {
     let t = TestContext::new_bob().await;