Protections can ban mods for ban/redact events

[BrenBarn]

It appears that the flood protection does not check to determine who is sending the event and/or what type of event it is. This means that if a room mod sends redact or ban events "manually" (e.g., via their client, without going through Mjolnir), the bot may ban them for "spamming".

Steps to reproduce the behavior:
Be a mod in a room. Have some other users send a bunch of messages. Then go into the Element side panel and ban the users and/or redact their messages. If you do this too many times too quickly, you will get banned by the bot.

Expected behavior
The automated protections should never ban mods or admins from the room.

Probably also admin-type events like bans or redactions should not "count" for purposes of determining whether a user is "flooding". Might need to think about what event types should count. (We probably want to protect against things like a user spamming by repeatedly changing their display name or whatever.)

[Yoric]

Oh, good point.

Would you be interested in working on fixing this?

[BrenBarn]

I don't think I can. I'm not familiar with the codebase and also am not really a JS/TS programmer (although I can pretty much read the code). Also it looks like there is a nontrivial build/test setup and I'm not really prepared to engage with that.

I took a look and it looks like the current system doesn't actually have any mechanism for checking the roles/permissions of users at all (only for the bot itself, to ensure that it can do its work). Is that correct?

[MichaelSasser]

Yeah, it bans for any kind of events, even if the powerlevel of the user is the same as the bots (100) (This claim is wrong, see #200 (comment)). I wasn't aware, that it is possible to ban someone with the same powerlevel as yours. If I remember that correctly, the spec does not allow this.

Workaround (if you trust your mods)

1. Demote the bot's powerlevel (e.g. 50).
2. Promote your mods to a powerlevel above the bot's (e.g. 60).
3. Make sure the bot has the permissions, it needs, including the modification of room ACLs. (Warning, this includes your mods permissions as well)

Now the bot isn't able to ban your mods. In case the protection gets triggered, it will throw a bunch of errors:

Mjolnir: Banning @your_mods_id:matrix.org in SomeRoom for flooding (28 messages in the last minute)
Mjolnir: There was an error processing an event through a protection - see log for details. Event: https://matrix.to/#/!foobarbaz:matrix.org/$0x4141foo-barbaz

The ban will not happen because the bot does not have the needed powerlevel to do that.
Even, if you added spam to automaticallyRedactForReasons, it will not redact the message in this case. Probably because of the previous error.

Not optimal, but a temporary fix.

[ShadowJonathan]

I wasn't aware, that it is possible to ban someone with the same powerlevel as yours. If I remember that correctly, the spec does not allow this.

It is not allowed by the state resolution, so it's likely @BrenBarn had a lower powerlevel than the bot.

[MichaelSasser]

It is not allowed by the state resolution, so it's likely @BrenBarn had a lower powerlevel than the bot.

Yes, in the scenario BrenBarn described, this is true. The bot had a powerlevel of 100, and BrenBarn had a powerlevel of 50.

What I was describing was a different scenario with the same outcome but under a false assumption. I was assuming the test-user triggering BasicFloodingProtection had the same powerlevel as the bot (powerlevel 100) in the room we tested in and the bot was able to ban the user. But after reviewing the logs, this was not the case. The powerlevel change happened after the user was banned, not before. After recreating the scenario, as it was intended, I was unable to replicate what I claimed before:

[...] [The bot banns] even if the powerlevel of the user is the same as the bots (100).

I run the (new) test with three kinds of events:

• m.room.message
• m.room.redaction
• m.reaction

The bot is not a homeserver admin and the test-user's and bot's accounts do not belong to the same homeserver.

As a result:

• the BasicFloodingProtection gets triggered by the described test-user, which is problematic, especially like in BrenBarn's scenario, where the bot has a higher powerlevel than BrenBarn
• the bot cannot ban the described user, which is to be expected
• the bot can ban a user with a lower powerlevel than the bots, which is to be expected

[BrenBarn]

I still think it would be a good idea (at least as an option) to prevent the bot from banning users with a certain power level. You might want to have mods in the room who can do some things (like kick/ban/redact) but not high-level admin actions (like changing the power level of other users), and you don't want those mods to get banned.

One way to do it would be to have the bot check the power level of the user it's about to ban, and not ban users who have sufficient power to ban other users.

[ShadowJonathan]

still think it would be a good idea (at least as an option) to prevent the bot from banning users with a certain power level.

This, i believe this is the cleanest solution.

[jesopo]

One way to do it would be to have the bot check the power level of the user it's about to ban, and not ban users who have sufficient power to ban other users.

this is a clean and simple solution, though i don't know if mjolnir passively holds enough state to know what someone's power level is without costly API calls. i'll try to figure this out

[JamesBelchamber]

It strikes me that a generic "ignore" list would be useful anyway (e.g. to add bots that look spammy but are alright), which could then be added to/removed from through rules (e.g. a power level change) as an additional feature. The list would be much easier to implement and would provide a bunch of functionality.

I note that an "ignore" list was also asked for in #321, for an entirely different purpose.

[jesopo]

It strikes me that a generic "ignore" list would be useful anyway (e.g. to add bots that look spammy but are alright), which could then be added to/removed from through rules (e.g. a power level change) as an additional feature. The list would be much easier to implement and would provide a bunch of functionality.

I note that an "ignore" list was also asked for in #321, for an entirely different purpose.

prior art on this; IRC anti-spam tooling ignores messages from people with even superficial access; e.g. give a bot a power level of 1 if you want it spared from automated spam checks

[MichaelSasser]

I was super excited about @anti-scam:matrix.org. Even though, I'm not entirely certain, where it comes from, and who runs it (maybe this five-year-old project?), but I was just waiting for it to get banned by mjolnir:

I mean, it's great, and really, really useful (actually, thanks to the people who made/run it ), but until this issue gets resolved, it will also be the victim of the flooding protection, every time. We can't give @anti-scam:matrix.org a higher powerlevel because it runs in an untrusted environment.

Workaround

1. Restart Mjolnir
2. /unban @anti-scam:matrix.org in the room, it was banned from
3. /invite @anti-scam:matrix.org int the room, it was banned from
4. goto 1, if mjolnir bans it again next time

[Mikaela]

I was super excited about @anti-scam:matrix.org. Even though, I'm not entirely certain, where it comes from, and who runs it (maybe this five-year-old project?), but I was just waiting for it to get banned by mjolnir:

https://github.com/jjj333-p/spam-police

[MTRNord]

(that anti-scam bot probably should get a rate limit to not react on every message in a wave but wait to the end and then react)

[H-Shay]

Hopefully fixed by #544