Skip to content

Commit 12fdfc9

Browse files
committed
fix: Don't resend webxdc status updates in OutBroadcast-s (#7679)
We don't remember which status updates are "initial" (sent initially by the broadcast owner) and which were sent by subscribers, so don't resend any of them to avoid accidental sharing of confidential data.
1 parent de0b23d commit 12fdfc9

5 files changed

Lines changed: 65 additions & 18 deletions

File tree

src/chat.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4682,7 +4682,7 @@ pub async fn resend_msgs(context: &Context, msg_ids: &[MsgId]) -> Result<()> {
46824682
// note(treefit): only matters if it is the last message in chat (but probably to expensive to check, debounce also solves it)
46834683
chatlist_events::emit_chatlist_item_changed(context, msg.chat_id);
46844684

4685-
if msg.viewtype == Viewtype::Webxdc {
4685+
if msg.viewtype == Viewtype::Webxdc && msg.chat_typ != Chattype::OutBroadcast {
46864686
let conn_fn = |conn: &mut rusqlite::Connection| {
46874687
let range = conn.query_row(
46884688
"SELECT IFNULL(min(id), 1), IFNULL(max(id), 0) \

src/constants.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,12 +107,14 @@ pub const DC_CHAT_ID_LAST_SPECIAL: ChatId = ChatId::new(9);
107107
IntoStaticStr,
108108
Serialize,
109109
Deserialize,
110+
Default,
110111
)]
111112
#[repr(u32)]
112113
pub enum Chattype {
113114
/// A 1:1 chat, i.e. a normal chat with a single contact.
114115
///
115116
/// Created by [`ChatId::create_for_contact`].
117+
#[default]
116118
Single = 100,
117119

118120
/// Group chat.

src/message.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -453,6 +453,7 @@ pub struct Message {
453453
pub(crate) is_dc_message: MessengerMessage,
454454
pub(crate) original_msg_id: MsgId,
455455
pub(crate) mime_modified: bool,
456+
pub(crate) chat_typ: Chattype,
456457
pub(crate) chat_visibility: ChatVisibility,
457458
pub(crate) chat_blocked: Blocked,
458459
pub(crate) location_id: u32,
@@ -527,6 +528,7 @@ impl Message {
527528
m.param AS param,
528529
m.hidden AS hidden,
529530
m.location_id AS location,
531+
c.type AS chat_typ,
530532
c.archived AS visibility,
531533
c.blocked AS blocked
532534
FROM msgs m
@@ -586,6 +588,10 @@ impl Message {
586588
param: row.get::<_, String>("param")?.parse().unwrap_or_default(),
587589
hidden: row.get("hidden")?,
588590
location_id: row.get("location")?,
591+
// This is safe: see `ChatId::delete_ex()`, `None` chat type can't happen.
592+
chat_typ: row
593+
.get::<_, Option<_>>("chat_typ")?
594+
.unwrap_or(Chattype::Single),
589595
chat_visibility: row.get::<_, Option<_>>("visibility")?.unwrap_or_default(),
590596
chat_blocked: row
591597
.get::<_, Option<Blocked>>("blocked")?

src/mimefactory.rs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2037,14 +2037,15 @@ impl MimeFactory {
20372037
HeaderDef::IrohGossipTopic.get_headername(),
20382038
mail_builder::headers::raw::Raw::new(topic).into(),
20392039
));
2040-
if let (Some(json), _) = context
2041-
.render_webxdc_status_update_object(
2042-
msg.id,
2043-
StatusUpdateSerial::MIN,
2044-
StatusUpdateSerial::MAX,
2045-
None,
2046-
)
2047-
.await?
2040+
if msg.chat_typ != Chattype::OutBroadcast
2041+
&& let (Some(json), _) = context
2042+
.render_webxdc_status_update_object(
2043+
msg.id,
2044+
StatusUpdateSerial::MIN,
2045+
StatusUpdateSerial::MAX,
2046+
None,
2047+
)
2048+
.await?
20482049
{
20492050
parts.push(context.build_status_update_part(&json));
20502051
}

src/webxdc/webxdc_tests.rs

Lines changed: 47 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1600,33 +1600,49 @@ async fn test_in_broadcast_send_status_update() -> Result<()> {
16001600
let qr = get_securejoin_qr(alice, Some(alice_chat_id)).await?;
16011601
let bob_chat_id = tcm.exec_securejoin_qr(bob, alice, &qr).await;
16021602

1603-
let alice_instance = send_webxdc_instance(alice, alice_chat_id).await?;
1603+
let mut alice_instance = create_webxdc_instance(
1604+
alice,
1605+
"minimal.xdc",
1606+
include_bytes!("../../test-data/webxdc/minimal.xdc"),
1607+
)?;
1608+
alice_chat_id
1609+
.set_draft(alice, Some(&mut alice_instance))
1610+
.await?;
1611+
alice_instance = alice_chat_id.get_draft(alice).await?.unwrap();
1612+
alice
1613+
.send_webxdc_status_update(alice_instance.id, r#"{"payload":41}"#)
1614+
.await?;
1615+
send_msg(alice, alice_chat_id, &mut alice_instance).await?;
16041616
let sent_msg = alice.pop_sent_msg().await;
16051617
let bob_instance = bob.recv_msg(&sent_msg).await;
16061618
assert_eq!(bob_instance.chat_id, bob_chat_id);
16071619

16081620
let provider = BackupProvider::prepare(bob).await?;
1609-
let bob1 = &tcm.unconfigured().await;
1610-
get_backup(bob1, provider.qr()).await?;
1621+
let bob2 = &tcm.unconfigured().await;
1622+
get_backup(bob2, provider.qr()).await?;
16111623

16121624
bob.send_webxdc_status_update(bob_instance.id, r#"{"payload":42}"#)
16131625
.await?;
16141626
bob.flush_status_updates().await?;
1615-
let sent_msg = bob.pop_sent_msg().await;
1627+
// Don't wait to make sure that status updates are sent immediately, otherwise the check for
1628+
// Alice below isn't reliable.
1629+
let sent_msg = bob.pop_sent_msg_opt(Duration::ZERO).await.unwrap();
16161630

16171631
alice.recv_msg_trash(&sent_msg).await;
16181632
assert_eq!(
16191633
alice
16201634
.get_webxdc_status_updates(alice_instance.id, StatusUpdateSerial(0))
16211635
.await?,
1622-
r#"[{"payload":42,"serial":1,"max_serial":1}]"#
1636+
r#"[{"payload":41,"serial":1,"max_serial":2},
1637+
{"payload":42,"serial":2,"max_serial":2}]"#
16231638
);
16241639

1625-
bob1.recv_msg_trash(&sent_msg).await;
1640+
bob2.recv_msg_trash(&sent_msg).await;
16261641
assert_eq!(
1627-
bob1.get_webxdc_status_updates(bob_instance.id, StatusUpdateSerial(0))
1642+
bob2.get_webxdc_status_updates(bob_instance.id, StatusUpdateSerial(0))
16281643
.await?,
1629-
r#"[{"payload":42,"serial":1,"max_serial":1}]"#
1644+
r#"[{"payload":41,"serial":1,"max_serial":2},
1645+
{"payload":42,"serial":2,"max_serial":2}]"#
16301646
);
16311647

16321648
// Non-subscriber's status updates are rejected.
@@ -1635,7 +1651,29 @@ async fn test_in_broadcast_send_status_update() -> Result<()> {
16351651
alice.pop_sent_msg().await;
16361652
let status =
16371653
helper_send_receive_status_update(bob, alice, &bob_instance, &alice_instance).await?;
1638-
assert_eq!(status, r#"[{"payload":42,"serial":1,"max_serial":1}]"#);
1654+
assert_eq!(
1655+
status,
1656+
r#"[{"payload":41,"serial":1,"max_serial":2},
1657+
{"payload":42,"serial":2,"max_serial":2}]"#
1658+
);
1659+
1660+
// Subscribers' status updates are confidential and shalln't be re-sent. So initial status
1661+
// updates aren't re-sent too.
1662+
let fiona = &tcm.fiona().await;
1663+
let fiona_chat_id = tcm.exec_securejoin_qr(fiona, alice, &qr).await;
1664+
resend_msgs(alice, &[alice_instance.id]).await?;
1665+
let sent1 = alice.pop_sent_msg().await;
1666+
alice.flush_status_updates().await?;
1667+
assert!(alice.pop_sent_msg_opt(Duration::ZERO).await.is_none());
1668+
let fiona_instance = fiona.recv_msg(&sent1).await;
1669+
assert_eq!(fiona_instance.chat_id, fiona_chat_id);
1670+
assert_eq!(fiona_instance.chat_typ, Chattype::InBroadcast);
1671+
assert_eq!(
1672+
fiona
1673+
.get_webxdc_status_updates(fiona_instance.id, StatusUpdateSerial(0))
1674+
.await?,
1675+
r#"[]"#
1676+
);
16391677
Ok(())
16401678
}
16411679

0 commit comments

Comments
 (0)