fix: validate Group IDs and SecureJoin tokens

Author: link2xt

spec.md

@@ -119,8 +119,9 @@ All group members form the member list.
 To allow different groups with the same members,
 groups are identified by a group-id.
 The group-id MUST be created only from the characters
-`0`-`9`, `A`-`Z`, `a`-`z` `_` and `-`
-and MUST have a length of at least 11 characters.
+`0`-`9`, `A`-`Z`, `a`-`z` `_` and `-`,
+MUST have a length of at least 11 characters
+and no more than 32 characters.
 
 Groups MUST have a group-name.
 The group-name is any non-zero-length UTF-8 string.

src/qr.rs

@@ -23,6 +23,7 @@ use crate::message::Message;
 use crate::peerstate::Peerstate;
 use crate::socks::Socks5Config;
 use crate::token;
+use crate::tools::validate_id;
 
 const OPENPGP4FPR_SCHEME: &str = "OPENPGP4FPR:"; // yes: uppercase
 const IDELTACHAT_SCHEME: &str = "https://i.delta.chat/#";
@@ -345,9 +346,18 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result<Qr> {
         "".to_string()
     };
 
-    let invitenumber = param.get("i").map(|s| s.to_string());
-    let authcode = param.get("s").map(|s| s.to_string());
-    let grpid = param.get("x").map(|s| s.to_string());
+    let invitenumber = param
+        .get("i")
+        .filter(|&s| validate_id(s))
+        .map(|s| s.to_string());
+    let authcode = param
+        .get("s")
+        .filter(|&s| validate_id(s))
+        .map(|s| s.to_string());
+    let grpid = param
+        .get("x")
+        .filter(|&s| validate_id(s))
+        .map(|s| s.to_string());
 
     let grpname = if grpid.is_some() {
         if let Some(encoded_name) = param.get("g") {
@@ -1035,6 +1045,21 @@ mod tests {
         Ok(())
     }
 
+    #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+    async fn test_decode_openpgp_invalid_token() -> Result<()> {
+        let ctx = TestContext::new().await;
+
+        // Token cannot contain "/"
+        let qr = check_qr(
+            &ctx.ctx,
+            "OPENPGP4FPR:79252762C34C5096AF57958F4FC3D21A81B0F0A7#a=cli%40deltachat.de&g=test%20%3F+test%20%21&x=h-0oKQf2CDK&i=9JEXlxAqGM0&s=0V7LzL/cxRL"
+        ).await?;
+
+        assert!(matches!(qr, Qr::FprMismatch { .. }));
+
+        Ok(())
+    }
+
     #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
     async fn test_decode_openpgp_secure_join() -> Result<()> {
         let ctx = TestContext::new().await;

src/receive_imf.rs

@@ -38,7 +38,9 @@ use crate::simplify;
 use crate::sql;
 use crate::stock_str;
 use crate::sync::Sync::*;
-use crate::tools::{self, buf_compress, extract_grpid_from_rfc724_mid, strip_rtlo_characters};
+use crate::tools::{
+    self, buf_compress, extract_grpid_from_rfc724_mid, strip_rtlo_characters, validate_id,
+};
 use crate::{contact, imap};
 
 /// This is the struct that is returned after receiving one email (aka MIME message).
@@ -2356,7 +2358,10 @@ async fn apply_mailinglist_changes(
 }
 
 fn try_getting_grpid(mime_parser: &MimeMessage) -> Option<String> {
-    if let Some(optional_field) = mime_parser.get_header(HeaderDef::ChatGroupId) {
+    if let Some(optional_field) = mime_parser
+        .get_header(HeaderDef::ChatGroupId)
+        .filter(|s| validate_id(s))
+    {
         return Some(optional_field.clone());
     }
 

src/receive_imf/tests.rs

@@ -1693,7 +1693,7 @@ async fn test_in_reply_to_two_member_group() {
                  Subject: foo\n\
                  Message-ID: <message@example.org>\n\
                  Chat-Version: 1.0\n\
-                 Chat-Group-ID: foo\n\
+                 Chat-Group-ID: foobarbaz12\n\
                  Chat-Group-Name: foo\n\
                  Date: Sun, 22 Mar 2020 22:37:57 +0000\n\
                  \n\
@@ -1738,7 +1738,7 @@ async fn test_in_reply_to_two_member_group() {
                  Message-ID: <chatreply@example.org>\n\
                  In-Reply-To: <message@example.org>\n\
                  Chat-Version: 1.0\n\
-                 Chat-Group-ID: foo\n\
+                 Chat-Group-ID: foobarbaz12\n\
                  Date: Sun, 22 Mar 2020 22:37:57 +0000\n\
                  \n\
                  chat reply\n",

src/tools.rs

@@ -277,6 +277,14 @@ pub(crate) fn create_id() -> String {
         .collect()
 }
 
+/// Returns true if given string is a valid ID.
+///
+/// All IDs generated with `create_id()` should be considered valid.
+pub(crate) fn validate_id(s: &str) -> bool {
+    let alphabet = base64::alphabet::URL_SAFE.as_str();
+    s.chars().all(|c| alphabet.contains(c)) && s.len() > 10 && s.len() <= 32
+}
+
 /// Function generates a Message-ID that can be used for a new outgoing message.
 /// - this function is called for all outgoing messages.
 /// - the message ID should be globally unique
@@ -966,6 +974,27 @@ DKIM Results: Passed=true, Works=true, Allow_Keychange=true";
         assert_eq!(buf.len(), 11);
     }
 
+    #[test]
+    fn test_validate_id() {
+        for _ in 0..10 {
+            assert!(validate_id(&create_id()));
+        }
+
+        assert_eq!(validate_id("aaaaaaaaaaaa"), true);
+        assert_eq!(validate_id("aa-aa_aaaXaa"), true);
+
+        // ID cannot contain whitespace.
+        assert_eq!(validate_id("aaaaa aaaaaa"), false);
+        assert_eq!(validate_id("aaaaa\naaaaaa"), false);
+
+        // ID cannot contain "/", "+".
+        assert_eq!(validate_id("aaaaa/aaaaaa"), false);
+        assert_eq!(validate_id("aaaaaaaa+aaa"), false);
+
+        // Too long ID.
+        assert_eq!(validate_id("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"), false);
+    }
+
     #[test]
     fn test_create_id_invalid_chars() {
         for _ in 1..1000 {

src/update_helper.rs

@@ -164,7 +164,7 @@ mod tests {
                  To: alice@example.org\n\
                  Message-ID: <msg1@example.org>\n\
                  Chat-Version: 1.0\n\
-                 Chat-Group-ID: abcde\n\
+                 Chat-Group-ID: abcde123456\n\
                  Chat-Group-Name: initial name\n\
                  Date: Sun, 22 Mar 2021 01:00:00 +0000\n\
                  \n\
@@ -182,7 +182,7 @@ mod tests {
                  To: alice@example.org\n\
                  Message-ID: <msg3@example.org>\n\
                  Chat-Version: 1.0\n\
-                 Chat-Group-ID: abcde\n\
+                 Chat-Group-ID: abcde123456\n\
                  Chat-Group-Name: another name update\n\
                  Chat-Group-Name-Changed: a name update\n\
                  Date: Sun, 22 Mar 2021 03:00:00 +0000\n\
@@ -197,7 +197,7 @@ mod tests {
                  To: alice@example.org\n\
                  Message-ID: <msg2@example.org>\n\
                  Chat-Version: 1.0\n\
-                 Chat-Group-ID: abcde\n\
+                 Chat-Group-ID: abcde123456\n\
                  Chat-Group-Name: a name update\n\
                  Chat-Group-Name-Changed: initial name\n\
                  Date: Sun, 22 Mar 2021 02:00:00 +0000\n\