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

Synapse doesn't accept m.room.power_level events with string values even though it sends them #12538

Closed
t3chguy opened this issue Apr 25, 2022 · 12 comments
Labels
T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.

Comments

@t3chguy
Copy link
Member

t3chguy commented Apr 25, 2022

Description

Synapse will happily send clients malformed m.room.power_level events with string power levels yet will not accept them, causing stuck PLs in a room where the client shouldn't be expected to have to sanitise the event which it was sent.

Steps to reproduce

  • Have a room with a power level event giving someone PL "0"
  • Try to update the power level of someone else

More context element-hq/element-web#19751 (comment)
Related #12537 (same cause)

Version information

  • Homeserver: matrix.org
@richvdh
Copy link
Member

richvdh commented Apr 26, 2022

I'm not really sure what we should do about this, tbh.

Obviously it's caused by the validation in #10232. On the one hand, it seems obvious that we shouldn't allow clients to send malformed PL events. On the other, if the PL event is malformed to start with, it's maybe not reasonable to require clients to "fix" it.

But attempting to "fix" the event before we pass it on to clients doesn't feel good either.

Maybe we should allow clients to send malformed PL events if the original PL event is malformed?

@richvdh
Copy link
Member

richvdh commented Apr 26, 2022

(Maybe we should just fix #12537 and then tell everyone they have to upgrade to a modern room version...)

@H-Shay H-Shay added X-Needs-Discussion T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. labels Apr 26, 2022
@dom96
Copy link

dom96 commented Apr 26, 2022

While you all resolve this, is there any workaround we can use for our channel? I need to be able to give mod rights to others as it's currently just me and I am not always online to moderate.

@richvdh
Copy link
Member

richvdh commented Apr 27, 2022

While you all resolve this, is there any workaround we can use for our channel?

use the /devtools to replace the strings in the PL event.

@erciccione
Copy link

use the /devtools to replace the strings in the PL event.

Worked for me. Was unable to give mod powers until i changed all strings into integers.

@sheerluck
Copy link

--- a/synapse/events/validator.py
+++ b/synapse/events/validator.py
@@ -94,6 +94,11 @@ class EventValidator:
                 )
 
         if event.type == EventTypes.PowerLevels:
+            users = event.content.get("users")
+            if users:
+                for key, val in users.items():
+                    if type(val) is str:
+                        users[key] = int(val)
             try:
                 jsonschema.validate(
                     instance=event.content,

@sheerluck
Copy link

Richard gave me "thumbs down" but 2 days ago in #12537 he wanted to pass them through int() himself :)

@dom96
Copy link

dom96 commented Apr 27, 2022

use the /devtools to replace the strings in the PL event.

Worked for me. Was unable to give mod powers until i changed all strings into integers.

After figuring out what /devtools meant it also worked for me, thanks! (for those that don't know and want to workaround as well: just type in /devtools into the Matrix chat box)

@richvdh
Copy link
Member

richvdh commented Apr 27, 2022

Richard gave me "thumbs down" but 2 days ago in #12537 he wanted to pass them through int() himself :)

In room upgrades. Not where you suggest.

@richvdh
Copy link
Member

richvdh commented Apr 28, 2022

(Maybe we should just fix #12537 and then tell everyone they have to upgrade to a modern room version...)

We'll wait until #12537 is fixed, but this is our current preferred approach

@MadLittleMods
Copy link
Contributor

#12537 was closed via #12657

Can we close this if our preferred approach is to force people to upgrade to a modern room version and that works now?

@DMRobertson
Copy link
Contributor

Let's do so.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
Development

No branches or pull requests

8 participants