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

Handle state pulled in via backfill more robustly #10794

Open
erikjohnston opened this issue Sep 10, 2021 · 2 comments
Open

Handle state pulled in via backfill more robustly #10794

erikjohnston opened this issue Sep 10, 2021 · 2 comments
Labels
A-Messages-Endpoint /messages client API endpoint (`RoomMessageListRestServlet`) (which also triggers /backfill) O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.

Comments

@erikjohnston
Copy link
Member

Synapse goes to great lengths to ensure that malicious servers cannot replace the current state of the room by sending an event E that points to all existing forward extremities plus a fake event that the server refuses to divulge. Naively, the server would ask for the state at E, which would then become the current state as E is the new forward extremity, instead Synapse rejects the event if the server refuses to divulge any of E's prev events. This ensures that the new current state cannot be replaced by malicious servers (without it being resolved against some existing state the server already has).

Currently, this logic only applies to newly received events, rather than events that come in via backfill. However, the same attack could be done via backfill in the same way. Thankfully, Synapse currently does not replace forwards extremities when events come in via backfill so Synapse isn't vulnerable to the attack, but that is very tenuous protection and we should add more explicit checks and protections.

@anoadragon453 anoadragon453 added the T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements. label Sep 13, 2021
@MadLittleMods MadLittleMods added the A-Messages-Endpoint /messages client API endpoint (`RoomMessageListRestServlet`) (which also triggers /backfill) label Aug 25, 2022
@MadLittleMods
Copy link
Contributor

Related to #10764

@DMRobertson
Copy link
Contributor

Is this detail important enough to be specced?

@DMRobertson DMRobertson added S-Tolerable Minor significance, cosmetic issues, low or no impact to users. O-Uncommon Most users are unlikely to come across this or unexpected workflow labels Aug 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Messages-Endpoint /messages client API endpoint (`RoomMessageListRestServlet`) (which also triggers /backfill) O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.
Projects
None yet
Development

No branches or pull requests

4 participants