Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MSC2676: Message editing #2676
MSC2676: Message editing #2676
Changes from 4 commits
8b95ffc
7f42c8b
5473009
e44f566
634dc2e
f9768e7
1de6c11
adcdddc
fc2a690
80c467b
6cea76a
dac2399
b8a7745
79a362e
bb96694
dd1ca71
eba4753
78550a2
e58715c
4c77c01
1850fb5
c9e6652
1e63094
2373873
8ec4cf3
08489e8
f3d328d
b245761
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not validating edits on the homeserver causes edits to be a super easy footgun for anyone writing a client or application-service. Someone can exploit this and edit anyone's message if you implement edits naively and expect the homeserver to validate edits like you can for redactions. We have first-hand experience of getting this wrong this in all of the Element clients 🥴
When exploitable, for native Matrix clients, it's most likely just a display problem since fixing the exploit on the client, results in the errant edit event being ignored and the original event shows up again or maybe even not a problem at all because Synapse filters them out of bundled data (not sure on exact Synapse details). But for bridges (application services) it can be a much bigger problem! Those edit events are not filtered out and get sent straight to the application service via
/transactions
. When someone naively relies on edit events to be correctly permissible against the room, those events get accidentally processed by the bridge and the information is persisted and overwritten in the 3rd party service.Validating edits with a power-level like redactions is a pretty good way forward though ⏩
matrix-org/synapse#5364 tracks the same problem space on Synapse
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've attempted to call out the dangers of trusting the edit events. For now, that's the framework we need to work within. If that's unworkable, it's going to need another round of changes to implementations and spec, so it's out of scope here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see a note where "Client authors are reminded to take note of the requirements for Validity of message edit events, and to ignore any invalid edit events that may be received." But this doesn't really call out any danger. It's such a subtle note pointing you to a list of ways you can get it wrong.
It seems like we should at least plainly point out these examples out in the security considerations section to explain what can happen.
And maybe a note about a future consideration to have it under a power-level.