Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Default state_default of missing m.room.power_levels event is incorrect #861

Closed
kegsay opened this issue Jul 14, 2021 · 1 comment · Fixed by #1479
Closed

Default state_default of missing m.room.power_levels event is incorrect #861

kegsay opened this issue Jul 14, 2021 · 1 comment · Fixed by #1479
Labels
spec-bug Something which is in the spec, but is wrong

Comments

@kegsay
Copy link
Member

kegsay commented Jul 14, 2021

In the distant past, state_default was 0 if the PL event was missing. https://github.com/matrix-org/synapse/blob/v0.18.5/synapse/api/auth.py#L997

This got changed in matrix-org/synapse@5c9afd6 to address room hijacking if there was no PL event.

The spec however hasn't been updated and still has the scary warnings about state_default being 0. Specifically, these statements lie:

If the room contains no m.room.power_levels event, both the state_default and events_default are 0.

Note: As noted above, in the absence of an m.room.power_levels event, the state_default is 0
@kegsay kegsay added the clarification An area where the expected behaviour is understood, but the spec could do with being more explicit label Jul 14, 2021
@turt2live turt2live added spec-bug Something which is in the spec, but is wrong and removed clarification An area where the expected behaviour is understood, but the spec could do with being more explicit labels Jul 14, 2021
@richvdh
Copy link
Member

richvdh commented Jul 15, 2021

links:

the spec text in question is at https://matrix.org/docs/spec/client_server/r0.6.1#m-room-power-levels.

this was changed in MSC1304 and implemented in matrix-org/synapse#3397. The spec should have been updated by matrix-org/matrix-spec-proposals#1656 but looks like this bit got forgotten.

@richvdh richvdh transferred this issue from matrix-org/matrix-spec-proposals Mar 2, 2022
@richvdh richvdh mentioned this issue Mar 31, 2023
Merged
richvdh added a commit that referenced this issue Apr 5, 2023
@richvdh
Remove outdated text saying that state_default defaults to 0 (#1479)
91b2f77 
There was substantial confusion around this, but I've done some archaeology.

Basically, this was changed back in r0.5.0 by MSC1304 and matrix-org/synapse#3397. Before that, it was indeed the case that state_default was 0 if there was no m.room.power_levels event, but that was confusing and a source of security holes, so we changed it.

matrix-org/matrix-spec-proposals#1656 changed the spec, but apparently overlooked the text in the description.

Reverts: #1478.
Fixes: #861.
clokep pushed a commit to clokep/matrix-spec that referenced this issue May 3, 2023
@richvdh @clokep
Remove outdated text saying that state_default defaults to 0 (matrix-…
2fdc634 
…org#1479)

There was substantial confusion around this, but I've done some archaeology.

Basically, this was changed back in r0.5.0 by MSC1304 and matrix-org/synapse#3397. Before that, it was indeed the case that state_default was 0 if there was no m.room.power_levels event, but that was confusing and a source of security holes, so we changed it.

matrix-org/matrix-spec-proposals#1656 changed the spec, but apparently overlooked the text in the description.

Reverts: matrix-org#1478.
Fixes: matrix-org#861.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
spec-bug Something which is in the spec, but is wrong
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove outdated text saying that state_default defaults to 0 matrix-org/matrix-spec
3 participants
@turt2live @richvdh @kegsay