From 956519cd98b9a7cd454738efb4fbbc809fc1d043 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Mon, 11 Aug 2025 15:22:55 +0200 Subject: [PATCH] fix: Make sure that only the channel owner can write into the chat --- src/chat/chat_tests.rs | 114 +++++++++++++++++++++++++++++++++++++++++ src/mimefactory.rs | 1 + src/receive_imf.rs | 19 +++++++ 3 files changed, 134 insertions(+) diff --git a/src/chat/chat_tests.rs b/src/chat/chat_tests.rs index be630e85f..6185aabf6 100644 --- a/src/chat/chat_tests.rs +++ b/src/chat/chat_tests.rs @@ -1,3 +1,5 @@ +use std::sync::Arc; + use super::*; use crate::chatlist::get_archived_cnt; use crate::constants::{DC_GCL_ARCHIVED_ONLY, DC_GCL_NO_SPECIALS}; @@ -3144,6 +3146,118 @@ async fn test_leave_broadcast_multidevice() -> Result<()> { Ok(()) } +/// Test that only the owner of the broadcast channel +/// can send messages into the chat. +/// +/// To do so, we change Alice's public key on Bob's side, +/// so that she is supposed to appear as a new contact when we receive another message, +/// and check that she can't write into the channel. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_only_broadcast_owner_can_send_1() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let bob = &tcm.bob().await; + + tcm.section("Alice creates broadcast channel and creates a QR code."); + let alice_chat_id = create_broadcast(alice, "foo".to_string()).await?; + let qr = get_securejoin_qr(alice, Some(alice_chat_id)).await.unwrap(); + + tcm.section("Bob now scans the QR code sends the request message"); + let bob_broadcast_id = join_securejoin(bob, &qr).await.unwrap(); + let request = bob.pop_sent_msg().await; + alice.recv_msg_trash(&request).await; + + tcm.section("Alice answers"); + let answer = alice.pop_sent_msg().await; + + tcm.section("Change Alice's fingerprint for Bob, so that she is a different contact from Bob's point of view"); + let bob_alice_id = bob.add_or_lookup_contact_no_key(alice).await.id; + bob.sql + .execute( + "UPDATE contacts + SET fingerprint='1234567890123456789012345678901234567890' + WHERE id=?", + (bob_alice_id,), + ) + .await?; + + tcm.section("Bob receives an answer, but it ignored because of a fingerprint mismatch"); + bob.recv_msg(&answer).await; + assert!( + load_broadcast_shared_secret(bob, bob_broadcast_id) + .await? + .is_none() + ); + + Ok(()) +} + +/// Same as the previous test, but Alice's fingerprint is changed later, +/// so that we can check that until the fingerprint change, everything works fine. +/// +/// Also, this changes Alice's fingerprint in Alice's database, rather than Bob's database, +/// in order to test for the same thing in different ways. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_only_broadcast_owner_can_send_2() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let bob = &mut tcm.bob().await; + + tcm.section("Alice creates broadcast channel and creates a QR code."); + let alice_broadcast_id = create_broadcast(alice, "foo".to_string()).await?; + let qr = get_securejoin_qr(alice, Some(alice_broadcast_id)) + .await + .unwrap(); + + tcm.section("Bob now scans the QR code"); + let bob_broadcast_id = join_securejoin(bob, &qr).await.unwrap(); + let request = bob.pop_sent_msg().await; + alice.recv_msg_trash(&request).await; + let answer = alice.pop_sent_msg().await; + + tcm.section("Bob receives an answer, and processes it"); + let rcvd = bob.recv_msg(&answer).await; + assert!( + load_broadcast_shared_secret(bob, bob_broadcast_id) + .await? + .is_some() + ); + assert_eq!(rcvd.param.get_cmd(), SystemMessage::MemberAddedToGroup); + + tcm.section("Alice sends a message, which still arrives fine"); + let sent = alice.send_text(alice_broadcast_id, "Hi").await; + let rcvd = bob.recv_msg(&sent).await; + assert_eq!(rcvd.text, "Hi"); + + tcm.section("Now, Alice's fingerprint changes"); + + alice.sql.execute("DELETE FROM keypairs", ()).await?; + alice + .sql + .execute("DELETE FROM config WHERE keyname='key_id'", ()) + .await?; + // Invalidate cached self fingerprint: + Arc::get_mut(&mut bob.ctx.inner) + .unwrap() + .self_fingerprint + .take(); + + tcm.section("Alice sends a message, which doesn't arrive fine"); + let sent = alice.send_text(alice_broadcast_id, "Hi").await; + let rcvd = bob.recv_msg(&sent).await; + assert_eq!( + rcvd.text, + "[Error: This message was not sent by the channel owner]" + ); + assert_eq!( + rcvd.error.unwrap(), + r#"Error: This message was not sent by the channel owner: +"Hi""# + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_encrypt_decrypt_broadcast() -> Result<()> { let mut tcm = TestContextManager::new(); diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 4ec123cb4..65eaa3139 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -584,6 +584,7 @@ impl MimeFactory { || step == "vb-member-added" || step == "vc-contact-confirm" // TODO possibly add vb-member-added here + // TODO wait... for member-added messages, Param::Arg doesn't even contain the step, but the email } } diff --git a/src/receive_imf.rs b/src/receive_imf.rs index f161fee0e..4c34b9ffd 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -1698,6 +1698,15 @@ async fn add_parts( for part in &mut mime_parser.parts { part.param.set(Param::OverrideSenderDisplayname, name); } + + if chat.typ == Chattype::InBroadcast { + let s = stock_str::error(context, "This message was not sent by the channel owner") + .await; + if let Some(part) = mime_parser.parts.first_mut() { + part.error = Some(format!("{s}:\n\"{}\"", part.msg)); + } + mime_parser.replace_msg_by_error(&s); + } } } @@ -3540,6 +3549,16 @@ async fn apply_in_broadcast_changes( ) -> Result { ensure!(chat.typ == Chattype::InBroadcast); + if let Some(part) = mime_parser.parts.first() { + if let Some(error) = &part.error { + warn!( + context, + "Not applying broadcast changes from message with error: {error}" + ); + return Ok(GroupChangesInfo::default()); + } + } + let mut send_event_chat_modified = false; let mut better_msg = None;