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

Serverside validation for m.room.power_levels event. (SYN-120) #1237

Closed
matrixbot opened this issue Oct 30, 2014 · 11 comments · Fixed by #10232
Closed

Serverside validation for m.room.power_levels event. (SYN-120) #1237

matrixbot opened this issue Oct 30, 2014 · 11 comments · Fixed by #10232
Labels
z-bug (Deprecated Label) z-p2 (Deprecated Label)

Comments

@matrixbot
Copy link
Member

Serverside validation for m.room.power_levels events as they an unholy mix of integers and strings in some channels

(Imported from https://matrix.org/jira/browse/SYN-120)

(Reported by @ara4n)

@matrixbot
Copy link
Member Author

Jira watchers: @NegativeMjark @ara4n

@matrixbot
Copy link
Member Author

Taken from "Matrix Internal".

{
  "@​LeoNerd:matrix.org": 100,
  "@​Mjark:matrix.org": "100",
  "@​dave:matrix.org": "100",
  "@​erikj:jki.re": "100",
  "@​kegan:matrix.org": "100",
  "@​matthew:matrix.org": "100",
  "@​neb:matrix.org": 100,
  "@​oddvar:matrix.org": 99,
  "default": 0
}

Possibly due differences between Leo's perl client and the web client? We should probably have better C2S and S2S validation for the event content.

-- @NegativeMjark

@matrixbot matrixbot added z-p2 (Deprecated Label) z-bug (Deprecated Label) labels Nov 7, 2016
@matrixbot matrixbot changed the title Serverside validation for m.room.power_levels event. (SYN-120) Serverside validation for m.room.power_levels event. (https://github.com/matrix-org/synapse/issues/1237) Nov 7, 2016
@matrixbot matrixbot changed the title Serverside validation for m.room.power_levels event. (https://github.com/matrix-org/synapse/issues/1237) Serverside validation for m.room.power_levels event. (SYN-120) Nov 7, 2016
@Half-Shot
Copy link
Collaborator

Can we shoot this in the head for the next version bump?

@erdnaxeli
Copy link
Contributor

erdnaxeli commented Jun 2, 2020

Having a power level being a string instead of an int makes the /upgraderoom code crash:

2020-05-28 15:36:01,839 - synapse.http.server - 110 - ERROR - POST-101101 - Failed handle request via 'Roo
mUpgradeRestServlet': <XForwardedForRequest at 0x7fe1efbe6978 method='POST' uri='/_matrix/client/r0/rooms/!gRrCzddQslIHoyrgwJ%3Amatrix.org/upgrade' clientproto='HTTP/1.1' site=8008>
Traceback (most recent call last):
  File "/opt/venvs/matrix-synapse/lib/python3.7/site-packages/twisted/internet/defer.py", line 1418, in _inlineCallbacks
    result = g.send(result)
StopIteration: {('m.room.power_levels', ''): '$159067103139tJGGs:cervoi.se', ('m.room.topic', ''): '$15005781984112702txYYW:matrix.org', ('m.room.history_visibility', ''): '$1500301245224694DtwYG:matrix.org', ('m.room.avatar', ''): '$15005773424098264pQgLH:matrix.org', ('m.room.guest_access', ''): '$1500301169223450JVurV:matrix.org', ('m.room.name', ''): '$15005781124111252eKoVg:matrix.org', ('m.room.join_rules', ''): '$1500301245224695ZMvZw:matrix.org'}

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/opt/venvs/matrix-synapse/lib/python3.7/site-packages/twisted/internet/defer.py", line 1418, in _inlineCallbacks
    result = g.send(result)
StopIteration: {('m.room.power_levels', ''): '$159067103139tJGGs:cervoi.se', ('m.room.topic', ''): '$15005781984112702txYYW:matrix.org', ('m.room.history_visibility', ''): '$1500301245224694DtwYG:matrix.org', ('m.room.avatar', ''): '$15005773424098264pQgLH:matrix.org', ('m.room.guest_access', ''): '$1500301169223450JVurV:matrix.org', ('m.room.name', ''): '$15005781124111252eKoVg:matrix.org', ('m.room.join_rules', ''): '$1500301245224695ZMvZw:matrix.org'}

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/opt/venvs/matrix-synapse/lib/python3.7/site-packages/synapse/http/server.py", line 78, in wrapped_request_handler
    await h(self, request)
  File "/opt/venvs/matrix-synapse/lib/python3.7/site-packages/synapse/http/server.py", line 331, in _async_render
    callback_return = await callback_return
  File "/opt/venvs/matrix-synapse/lib/python3.7/site-packages/synapse/rest/client/v2_alpha/room_upgrade_rest_servlet.py", line 76, in on_POST
    requester, room_id, new_version
  File "/opt/venvs/matrix-synapse/lib/python3.7/site-packages/synapse/handlers/room.py", line 146, in upgrade_room
    new_version,  # args for _upgrade_room
  File "/opt/venvs/matrix-synapse/lib/python3.7/site-packages/twisted/internet/defer.py", line 1418, in _inlineCallbacks
    result = g.send(result)
  File "/opt/venvs/matrix-synapse/lib/python3.7/site-packages/synapse/handlers/room.py", line 195, in _upgrade_room
    tombstone_event_id=tombstone_event.event_id,
  File "/opt/venvs/matrix-synapse/lib/python3.7/site-packages/synapse/handlers/room.py", line 378, in clone_existing_room
    initial_state[(EventTypes.PowerLevels, "")]
  File "/opt/venvs/matrix-synapse/lib/python3.7/site-packages/synapse/events/utils.py", line 445, in copy_power_levels_contents
    "Invalid power_levels value for %s.%s: %r" % (k, k1, v1)
TypeError: Invalid power_levels value for users.@fwhcat:mux.re: '100'

@melroy89
Copy link

Indeed same error:

2020-12-24 22:21:51,461 - synapse.http.server - 79 - ERROR - POST-91167 - Failed handle request via 'RoomUpgradeRestServlet': <XForwardedForRequest at 0x7f823f0dda60 method='POST' uri='/_matrix/client/r0/rooms/!secret_room_id%3Amatrix.org/upgrade' clientproto='HTTP/1.0' site=8008>
Traceback (most recent call last):
  File "/usr/local/lib/python3.8/site-packages/synapse/http/server.py", line 224, in _async_render_wrapper
    callback_return = await self._async_render(request)
  File "/usr/local/lib/python3.8/site-packages/synapse/http/server.py", line 400, in _async_render
    callback_return = await raw_callback_return
  File "/usr/local/lib/python3.8/site-packages/synapse/rest/client/v2_alpha/room_upgrade_rest_servlet.py", line 76, in on_POST
    new_room_id = await self._room_creation_handler.upgrade_room(
  File "/usr/local/lib/python3.8/site-packages/synapse/handlers/room.py", line 165, in upgrade_room
    ret = await self._upgrade_response_cache.wrap(
  File "/usr/local/lib/python3.8/site-packages/twisted/internet/defer.py", line 1418, in _inlineCallbacks
    result = g.send(result)
  File "/usr/local/lib/python3.8/site-packages/synapse/handlers/room.py", line 223, in _upgrade_room
    await self.clone_existing_room(
  File "/usr/local/lib/python3.8/site-packages/synapse/handlers/room.py", line 411, in clone_existing_room
    ] = power_levels = copy_power_levels_contents(
  File "/usr/local/lib/python3.8/site-packages/synapse/events/utils.py", line 446, in copy_power_levels_contents
    raise TypeError(
TypeError: Invalid power_levels value for users.@purple_azurite:matrix.org: '0'

Could be fixed easily by the built-in Python: int()

@grinapo
Copy link

grinapo commented Jan 30, 2021

7 years gone by

@avdb13
Copy link

avdb13 commented Feb 15, 2022

power

In addition to this negative power levels aren't evaluated properly and can be set as low as I want when it shouldn't be able to be smaller than -1.

@aaronraimist
Copy link
Contributor

@avdb13 what makes you think it can't be smaller than -1? The spec says integers can be between -9007199254740991 and 9007199254740991 https://spec.matrix.org/v1.2/appendices/#canonical-json

@avdb13
Copy link

avdb13 commented Feb 18, 2022

@aaronraimist everything lower than -1 doesn't have any effect therefore it shouldn't be allowed. I wonder why the Matrix spec uses a range from 0 to 100 for permissions, something like filesystem permissions on *NIX would be much more flexible since I currently can't give someone only permissions to change the room name without being able to kick people.

@grinapo
Copy link

grinapo commented Feb 18, 2022

@aaronraimist everything lower than -1 doesn't have any effect

Why would be so? If muted is -5000, default is -1000, admin is -250, superadmin is 0, then ir works just the same.

(Levels are not good idea in my opinion either since I cannot have people allowed to kick but not to change topic, then other people allowed to change topic but not to kick, but that's what we have now.)

@aaronraimist
Copy link
Contributor

aaronraimist commented Feb 18, 2022

Exactly. It has an effect if you want it to have an effect.

something like filesystem permissions on *NIX would be much more flexible

Synapse implements the Matrix spec. If you want to change how power levels work, you would have to change it in the spec, for example matrix-org/matrix-spec-proposals#2812.

I wonder why the Matrix spec uses a range from 0 to 100 for permissions

It doesn't.

I currently can't give someone only permissions to change the room name without being able to kick people.

That's not true. You can set the power level required to do those things to different values. https://spec.matrix.org/v1.2/client-server-api/#mroompower_levels

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
z-bug (Deprecated Label) z-p2 (Deprecated Label)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants