From 14aaab05b04c075d7f05d27939b1c16e9c34c95d Mon Sep 17 00:00:00 2001 From: bjoern Date: Fri, 10 May 2024 16:43:49 +0200 Subject: [PATCH] limit quote replies (#5543) this PR checks if the quotes are used in a reasonable way: - quoted messages should be send to the same chat - or to one-to-one chats if the quote's chat ID is not the same than the sending chat _and_ the sending chat is not a one-to-one chat, sending is aborted. usually, the UIs does not allow quoting in other ways, so, this check is only a "last defence line" to avoid leaking data in case the UI has bugs, as recently in https://github.com/deltachat/deltachat-android/issues/3032 @iequidoo @link2xt @adbenitez i am not 100% sure about this PR, maybe i've overseen a reasonable usecase where other quotes make sense --------- Co-authored-by: link2xt --- src/chat.rs | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/src/chat.rs b/src/chat.rs index 3020a1b95..7da941d31 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -2593,6 +2593,18 @@ async fn prepare_msg_common( } } + // Check a quote reply is not leaking data from other chats. + // This is meant as a last line of defence, the UI should check that before as well. + // (We allow Chattype::Single in general for "Reply Privately"; + // checking for exact contact_id will produce false positives when ppl just left the group) + if chat.typ != Chattype::Single && !context.get_config_bool(Config::Bot).await? { + if let Some(quoted_message) = msg.quoted_message(context).await? { + if quoted_message.chat_id != chat_id { + bail!("Bad quote reply"); + } + } + } + // check current MessageState for drafts (to keep msg_id) ... let update_msg_id = if msg.state == MessageState::OutDraft { msg.hidden = false; @@ -4713,6 +4725,59 @@ mod tests { Ok(()) } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn test_quote_replies() -> Result<()> { + let alice = TestContext::new_alice().await; + let bob = TestContext::new_bob().await; + + let grp_chat_id = create_group_chat(&alice, ProtectionStatus::Unprotected, "grp").await?; + let grp_msg_id = send_text_msg(&alice, grp_chat_id, "bar".to_string()).await?; + let grp_msg = Message::load_from_db(&alice, grp_msg_id).await?; + + let one2one_chat_id = alice.create_chat(&bob).await.id; + let one2one_msg_id = send_text_msg(&alice, one2one_chat_id, "foo".to_string()).await?; + let one2one_msg = Message::load_from_db(&alice, one2one_msg_id).await?; + + // quoting messages in same chat is okay + let mut msg = Message::new(Viewtype::Text); + msg.set_text("baz".to_string()); + msg.set_quote(&alice, Some(&grp_msg)).await?; + let result = send_msg(&alice, grp_chat_id, &mut msg).await; + assert!(result.is_ok()); + + let mut msg = Message::new(Viewtype::Text); + msg.set_text("baz".to_string()); + msg.set_quote(&alice, Some(&one2one_msg)).await?; + let result = send_msg(&alice, one2one_chat_id, &mut msg).await; + assert!(result.is_ok()); + let one2one_quote_reply_msg_id = result.unwrap(); + + // quoting messages from groups to one-to-ones is okay ("reply privately") + let mut msg = Message::new(Viewtype::Text); + msg.set_text("baz".to_string()); + msg.set_quote(&alice, Some(&grp_msg)).await?; + let result = send_msg(&alice, one2one_chat_id, &mut msg).await; + assert!(result.is_ok()); + + // quoting messages from one-to-one chats in groups is an error; usually this is also not allowed by UI at all ... + let mut msg = Message::new(Viewtype::Text); + msg.set_text("baz".to_string()); + msg.set_quote(&alice, Some(&one2one_msg)).await?; + let result = send_msg(&alice, grp_chat_id, &mut msg).await; + assert!(result.is_err()); + + // ... but forwarding messages with quotes is allowed + let result = forward_msgs(&alice, &[one2one_quote_reply_msg_id], grp_chat_id).await; + assert!(result.is_ok()); + + // ... and bots are not restricted + alice.set_config(Config::Bot, Some("1")).await?; + let result = send_msg(&alice, grp_chat_id, &mut msg).await; + assert!(result.is_ok()); + + Ok(()) + } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_add_contact_to_chat_ex_add_self() { // Adding self to a contact should succeed, even though it's pointless.