Page MenuHomePhabricator

Blocked users should not be allowed to run checkuser queries
Closed, ResolvedPublic

Description

When a user who has the checkuser rights is blocked he will still be able to run checkuser queries while being blocked from editing. Currently checkuser rights need to be removed to prevent access.

Thus, if a checkuser account is taken over (which of course itself is a worst-case-scenario), a bureaucrat+sysop might notice this, block the account and remove sysop-access to prevent unblockself, but might leave the checkuser-rights (for example because the local bureaucrat is able to remove sysop access but cannot remove checkuser access). The blocked user will still be able to run a query.

In addition to that on small wikis with just two checkusers nobody might notice that the blocked account runs queries if the second checkuser is not online, as the checkuser log is hidden.

Event Timeline

EddieGP created this task.Feb 11 2017, 9:36 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 11 2017, 9:36 PM
EddieGP updated the task description. (Show Details)Feb 11 2017, 9:41 PM
Isarra added a subscriber: Isarra.Feb 11 2017, 10:30 PM
Legoktm set Security to Software security bug.Feb 12 2017, 4:54 AM
Legoktm added a project: Security.
Legoktm changed the visibility from "Public (No Login Required)" to "Custom Policy".
Legoktm added a subscriber: Legoktm.
EddieGP claimed this task.Feb 12 2017, 8:23 PM

Found a patch, now I just need to work myself through the tutorials and configuring git to commit it...

MaxSem added a subscriber: MaxSem.Feb 12 2017, 9:26 PM

Please don't commit your fix publicly - attach a patch file to this bug so that it could be deployed to production before it goes public.

EddieGP added a comment.EditedFeb 12 2017, 9:41 PM

@MaxSem Sorry, I read this too late. Patch is up here:
https://gerrit.wikimedia.org/r/#/c/337326/

If there is any chance to remove this, please do so. This is literally my first bug report and commit ever, I don't know how to delete the patch from gerrit, or how I may add the patch file to this bug (just uploading the changed php file?)

Sorry :-(

Edit: I did "abandoned" but I believe this didn't solve it. Sorry again.

Well, it's public now. I would simply continue treating it as a normal patch. Thankfully, it's not something critical, as I don't think we have rogue checkusers blocked.

Also, it's a quite trivial fix, so it should be easily merged. I have left you some comments on the patch (style issues, actually).

Thanks for your bug report and patch!

@Platonides Thanks for your reply ... I was worried that I may have done something horribly wrong. I now removed the "abandoned" again (it seems this didn't help at all, still stayed public). And thanks for your annotations.

Dear all, sorry for the mess ...

EddieGP moved this task from Backlog to Patches in review on the CheckUser board.Feb 12 2017, 10:20 PM

Don't worry: nothing broke, nobody got hurt. Just know the procedure next time:)

EddieGP closed this task as Resolved.Feb 19 2017, 9:21 PM

Merged, though gerritbot doesn't mention this here, I guess the bot does not have permissions for security patches as those should not be on gerrit in the first place. Lesson learned. Thanks again for all the help!

@Aklapper @Legoktm Could somebody please remove the policy from this one again? It's been public in gerrit & the release notes by now ...

Once again, could anybody please remove the access policy here? This patch has gone public more than a month ago!

Jalexander changed the visibility from "Custom Policy" to "Public (No Login Required)".Mar 23 2017, 11:56 PM