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 <link2xt@testrun.org>
This commit is contained in:
bjoern
2024-05-10 16:43:49 +02:00
committed by GitHub
parent 72c09feb64
commit 14aaab05b0

View File

@@ -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.