Fork me on GitHub
Subscribe 3

Ticket #343 (fixed bug)

Moderators can ban other moderators

  • Created: 2011-03-10 14:14:37
  • Reported by: Franz
  • Assigned to: Franz
  • Milestone: 1.4.5
  • Component: security
  • Priority: high

It is currently possible for moderators (if they are allowed to ban users) to ban other moderators. This should not be allowed.



Reines 2011-03-11 09:29:25

  • Component set to security.

Franz 2011-03-12 14:37:09

Commit 7c4b93b to fluxbb fluxbb-1.4

#343: Moderators could ban other moderators.

Franz 2011-03-12 14:37:14

  • Status changed from open to fixed.

And fixed in 7c4b93b.

I will have to remember to do this in #168, too.

quy 2011-03-12 15:45:34

  • Status changed from fixed to open.

To reproduce, click the Ban button without entering an username.

File: C:\xampp\htdocs\fluxbb145master\admin_bans.php
Line: 62

FluxBB reported: Unable to fetch group info

Database reported: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '' at line 1 (Errno: 1064)

Failed query: SELECT g_moderator FROM groups WHERE g_id=

Reines 2011-03-12 15:48:25

Commit 3e7b8f4 to fluxbb fluxbb-1.4

Only check for moderator group if we have a group, fixes #343

Reines 2011-03-12 15:48:50

  • Status changed from open to fixed.

This should fix that, cheers.

Franz 2011-03-12 15:55:15

Ah, sorry. So that's why the isset() check was there in the code above...

Franz 2011-03-13 17:48:38

Ah, I think I forgot to hide the ban button for moderators on moderators' profiles. I will look into it.

Reines 2011-03-13 18:01:18

  • Status changed from fixed to open.

quy 2011-03-14 17:41:59

There are 2 other ways to do this on the Bans page.
1) Edit an existing ban entry
2) Add a ban entry but don't enter the moderator's username on the 1st page, but do it on the 2nd page.

Franz 2011-03-22 01:28:23

Commit 8e3c445 to fluxbb fluxbb-1.4

#343: Moderators could still ban other moderators. Banning moderators is now prohibited altogether.

Franz 2011-03-22 01:31:16

  • Status changed from open to fixed.

Ok, after Quy's recommendation I now prohibited banning moderators altogether in 8e3c445.

This could theoretically still be circumvented by banning the IP of a moderator (or administrator, for that matter). If you don't mind, I won't add this before 1.4.6, though.

quy 2011-03-22 04:05:55

In check_bans function, a partial/immediate solution is to add the default moderator group (PUN_MOD) to the check which would cover most installs.

    // Admins aren't affected
    if ($pun_user['g_id'] == PUN_ADMIN || !$pun_bans)

Franz 2011-03-22 13:02:30

Still wondering whether this would be good. Banning administrators won't work anyway, so in some (rare, weird) cases you might actually want to ban their old IP.

For moderators, things get complicated. We'd also need to decide which IP address to use (the newest, I suppose) and whether that would cause any damage.

Reines 2011-03-22 13:12:56

To be honest I wouldn't bother with worrying about banning IP addresses of moderators/admins, since due to dynamic IPs and so on it's basically impossible to make it work 100%.

quy 2011-03-22 13:17:12

Yes, because it does not make sense to ban an administrator/moderator and not move the banned user to a non administrator/moderator group.

Franz 2011-03-22 16:15:23

Well, Quy, the problem would be that "evil" moderators could try to ban another moderator via their IP even though it shouldn't be possible at all.

quy 2011-03-22 16:32:12

That is why I suggested adding the default moderator group to be excluded in the check_bans function.

Franz 2011-03-22 16:37:38

Gotcha, makes sense. Especially as I now prohibited banning moderators. I misunderstood your point earlier, sorry.

And thanks, I'll do that in a few minutes. smile

Franz 2011-03-22 16:45:49

Commit b4937b4 to fluxbb fluxbb-1.4

Bans won't work on moderators anymore.

Related to #343.

Franz 2011-03-22 16:52:38

Ok, I've committed b4937b4, which does this in a slightly simpler way and for all moderators.
So we should really be done with this now.

One more, only slightly related thing: expired bans are not deleted when admins or mods browse the board, due to the way the check_bans() function is constructed. Should we change this?

Reines 2011-03-22 18:51:25

Nah, they will be deleted when any non-admin/mod browses so that's good enough.