Skip to content

Auth rules for PL events don't say if the default value counts as "present" or not #2388

Description

@kegsay

Link to problem area:

https://spec.matrix.org/v1.17/rooms/v12/#authorization-rules

For each entry being changed in, or removed from, the events or notifications properties:

  • If the current value is greater than the sender’s current power level, reject.

For each entry being added to, or changed in, the events or notifications properties:

  • If the new value is greater than the sender’s current power level, reject.

Issue

What counts as "added"? There are two interpretations:

  • strict: any new key in the JSON map
  • loose: any semantic behaviour change. foo: absent → 30 with events_default = 30 is behaviourally the same, so not an "addition".

The loose reading is wrong, but the spec doesn't call this out. The problem with the loose reading is it allows unauthorised users to modify the PL event in two scenarios:

  • Assigning a specific event type's PL value: If events["foo"] was absent, and events_default: 30, then a user can set events["foo"] = 30 under the loose reading. Strict reading would forbid it, causing divergence.
  • Removing a specific event type's PL value: If events["m.room.name"] == 50 (it is set to this by default) and state_default: 50 (it is by default), then a user can unset events["m.room.name"] since semantically the PL doesn't change.

This can cause state to diverge:
- Strict servers will reject all subsequent PL events from loose servers due to the PL event referencing the previous PL event in auth_events, causing PL events to diverge.
- The mainline ordering uses PL events under the assumption that only authorised users can modify specific event values, which isn't true under the loose reading. This provides a mechanism for non-auth room state to diverge, as loose servers will see a much longer mainline than strict servers.

In practice this is quite hard to do because by default in order for the unauthorised user to no-op the events map they need to be authorised to send PL events, which by default is "m.room.power_levels": 100,, which likely limits the impact of this.

It's primarily an issue when implementations write things like:

prev_value = existing_state.get_or_default("some.type", 50)
next_value = pl_event.get_or_default("some.type", 50)
if prev_value != next_value {
    // permission checks
}

^ is wrong.

Metadata

Metadata

Assignees

No one assigned

    Labels

    clarificationAn area where the expected behaviour is understood, but the spec could do with being more explicit

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions