Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Invites from ignored users may not be ignored #11506

Closed
DMRobertson opened this issue Dec 3, 2021 · 1 comment · Fixed by #11511
Closed

Invites from ignored users may not be ignored #11506

DMRobertson opened this issue Dec 3, 2021 · 1 comment · Fixed by #11511
Labels
A-Spec-Compliance places where synapse does not conform to the spec T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.

Comments

@DMRobertson
Copy link
Contributor

DMRobertson commented Dec 3, 2021

Midway down the digestive system of Synapse's sync code, we fetch all membership changes for a user across the sync period.

After 1709, event will be the last element in the list of events, or it'll be unset if there were no such events. They're fetched in ascending stream ordering order, so event will be the membership change with highest stream ordering.

membership_change_events = await self.store.get_membership_changes_for_user(
user_id, since_token.room_key, now_token.room_key
)
mem_change_events_by_room_id: Dict[str, List[EventBase]] = {}
for event in membership_change_events:
mem_change_events_by_room_id.setdefault(event.room_id, []).append(event)

There are no other writes to event, but there is a read:

# Only bother if we're still currently invited
should_invite = non_joins[-1].membership == Membership.INVITE
if should_invite:
if event.sender not in ignored_users:
invite_room_sync = InvitedSyncResult(room_id, invite=non_joins[-1])
if invite_room_sync:
invited.append(invite_room_sync)

I smell a bug.

  • Suppose my membership_change_events consists of three events in this order:
    • I'm invited to room A by someone I've ignored
    • I'm invited to room B by someone I've not ignord.
  • event will be the invite event in room B.
  • mem_change_events_by_room_id will be the dictionary {A: [invite_A], B: [invite_B]}.
  • We start looping over the keys of this dictionary.
  • When I process room A, we test event.sender not in ignored users
  • **event refers invite_B, whose sender is not ignored. We then push invite_A down to the user.

Well this is just embarrassing. Should be an easy fix though.

@DMRobertson DMRobertson added the T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. label Dec 3, 2021
@DMRobertson
Copy link
Contributor Author

Note the spec says:

Invites to new rooms by ignored users will not be sent to the client.

DMRobertson pushed a commit to matrix-org/complement that referenced this issue Dec 3, 2021
@DMRobertson DMRobertson added the A-Spec-Compliance places where synapse does not conform to the spec label Dec 6, 2021
DMRobertson pushed a commit to matrix-org/complement that referenced this issue Dec 9, 2021
…cs (#267)

* Change SyncUntil* functions to return next_batch
* Add a matcher which checks that a key is missing
* Reproduce matrix-org/synapse#11506
* Blocklist this test on Dendrite
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Spec-Compliance places where synapse does not conform to the spec T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant