Skip to content

[state] Reaction event does not dedupe by author #111

@intendednull

Description

@intendednull

Parent: #108

Problem

crates/state/src/materialize.rs:383-390:

EventKind::Reaction { message_id, emoji } => {
    if let Some(msg) = state.messages.iter_mut().find(|m| m.id == *message_id) {
        msg.reactions
            .entry(emoji.clone())
            .or_default()
            .push(event.author);
    }
}

There is no check that event.author is already in the reactions list. The same author emitting two Reaction events for the same emoji on the same message produces duplicate entries, so clients display 👍 alice × 3 as 👍 alice, alice, alice, and state diverges across peers that saw different counts of duplicate events.

Exploit (reproduced)

Standalone binary: Alice posts a message, then emits two Reaction events with emoji = "thumbs". After materialize():

reactions['thumbs'].len() = 2
BUG CONFIRMED: same author double-reaction counted twice

Fix

Change the inner type from Vec<PeerId> to BTreeSet<PeerId>. This gives dedupe for free and preserves deterministic iteration order. Required changes:

  1. crates/state/src/types.rs — change the field type on ChatMessage::reactions.
  2. crates/state/src/materialize.rs:383-390 — change .push(event.author) to .insert(event.author).
  3. Anywhere that iterates msg.reactionsBTreeSet::iter() already yields items in sorted order, so rendering may need adjustment if any code assumed insertion order.
  4. willow-messaging / willow-client / willow-app / willow-web may have accessor code that consumes the old Vec shape — grep for .reactions and adjust.

Alternative (less invasive): keep the Vec<PeerId> shape and add a contains(&event.author) check before .push(). This is smaller but leaves the footgun in place for future authors of the code.

Recommend the BTreeSet change for safety.

Test

#[test]
fn same_author_duplicate_reaction_is_idempotent() {
    // post a message, react twice with same emoji from same author,
    // assert reactions[emoji].len() == 1
}

Regression test should mirror the exploit binary: two Reaction events from the same identity, same emoji, same target message, assert only one entry after materialization.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions