user with extremely long MXID causes mjolnir to fail to ban #322
Comments
|
I would recomend a edit to the Issue descript that replaces the offensive MXID with This new MXID is also 255 characters long but it is also complete junk data instead of a offensive MXID and it will trigger the exact same bug. Like from my prespective i dont rly see a reason why we need to have this MXID in the issue. The account was deactivated by the HS admin and well the issue isnt unique to this MXID its actually just Mjolnir today fails to handle 255 char MXIDs due to a state key size limit and the way the state key is constructed for policy. Below is some sanetised Logs of this exact bug being triggered using the MXID from the report btw. Sanetised as in i will replace the MXID with my pure ones version and i am changing the room ID |
|
The proper remedy is a spec change allowing long state keys and/or jumbo events. |
No. Nico (can't remember his github name) already discovered that this is likely a Synapse messup on the size check. That user probably shouldn't even exist. Iirc the state_key size limit is 255 bytes. So a logic step would be a) to check if the mxid length aligns with this and b) check how in the world that user even was able to join considering that a join would need a join event meaning that somehow that state_key length limit was evaded in the first place 🤔 |
|
Yes, state_key size limit is 255 bytes. |
|
No, the user is fine. The problem is that mjolnir doesn't account for the role: prefix when generating the state key. You can manually create the ban event in the banlist room with a shorter state key. See https://matrix.to/#/%23community-moderation-effort-bl%3Aneko.dev?via=neko.dev&via=jae.fi&via=grapheneos.org&via=matrix.org for example which bans that user too. |
|
(I would suggest mjolnir generates a random string for the last 127 chars of an mxid to avoid clashes when generating rules if the userid is too long) |
1. We keep track of the event that created a list rule so that we can remove the rule by having a way to determine the original state key for the rule. This is because the state key of rules can be anything and should not be relied on by Mjolnir to unban things (which it was doing). 2. The old scheme for producing a state key was causing for some entities to escape bans #322. We could have used a hash or something similar, but we know that the reason for the `rule:${entity}` scheme existed was for ease of debugging and finding rules in devtools. So instead we have followed a scheme simalar to bridges where the first character of an mxid is replaced with an underscore. Everything else just gets put into the state key. Since domains can't have '@' and room ids, aliases can't either. 3. We have stopped the need for Mjolnir to wait for the next response from sync after banning, unbanning an entity so that we can apply ACL's sooner.
1. We keep track of the event that created a list rule so that we can remove the rule by having a way to determine the original state key for the rule. This is because the state key of rules can be anything and should not be relied on by Mjolnir to unban things (which it was doing). 2. The old scheme for producing a state key was causing for some entities to escape bans #322. We could have used a hash or something similar, but we know that the reason for the `rule:${entity}` scheme existed was for ease of debugging and finding rules in devtools. So instead we have followed a scheme simalar to bridges where the first character of an mxid is replaced with an underscore. Everything else just gets put into the state key. Since domains can't have '@' and room ids, aliases can't either. 3. We have stopped the need for Mjolnir to wait for the next response from sync after banning, unbanning an entity so that we can apply ACL's sooner.
* Rework the banning and unbanning of entities in PolicyLists. 1. We keep track of the event that created a list rule so that we can remove the rule by having a way to determine the original state key for the rule. This is because the state key of rules can be anything and should not be relied on by Mjolnir to unban things (which it was doing). 2. The old scheme for producing a state key was causing for some entities to escape bans #322. We could have used a hash or something similar, but we know that the reason for the `rule:${entity}` scheme existed was for ease of debugging and finding rules in devtools. So instead we have followed a scheme simalar to bridges where the first character of an mxid is replaced with an underscore. Everything else just gets put into the state key. Since domains can't have '@' and room ids, aliases can't either. 3. We have stopped the need for Mjolnir to wait for the next response from sync after banning, unbanning an entity so that we can apply ACL's sooner. * Use PolicyList's `banEntity` method to create imported rules.
|
I believe this has been fixed. If not, please open a new issue. |
|
The 1.6.0 release fixes this so yes its correctly fixed as far as i remember. |
Describe the bug
mjolnir fails to ban a user with a extremely long MXID
To Reproduce
Steps to reproduce the behavior:
@aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:example.comExpected behavior
for the user to be banned:
There was an error processing your command - see console/log for details(issue edited to remove offensive mxid)
The text was updated successfully, but these errors were encountered: