Page MenuHomePhabricator

Limit the forwarding actions for Special:Random
Closed, ResolvedPublic

Description

As a code hardening effort, we should limit the forwarding actions of Special:Random. action=delete certainly, and potentially action=edit.

Details

Event Timeline

If I'm understanding this right, this seems rather round-about in relation to the incident. It sounds like the focus is being placed on the particular script that's been loaded, and its specific method of operating, i.e. POST requests on the regular, web UI edit forms. Does a theoretical solution to this task cover a script (which (a) in the first place has to be loaded by a user (b) can spread itself by editing site JS freely if the user is privileged enough) that simply goes for ApiQueryRandom followed by a form submission, or just the edit API?

This is definitely the wrong way of going about this. The issue happened because a malicious script was loaded that can execute any action in the context of the current user, and blocking actions from being forwarded for Special:Random would do nothing to deter a determined attacker from using other methods to achieve the same thing (e.g. what @Alex44019 said above.) This is like trying to patch an exploit by blocking fixed input signatures instead of fixing the underlying problem, which is that MediaWiki:common.js/gadgets/userscripts are able to do whatever they want without any sandboxing. The Web currently doesn't offer anything that can help with this, but proposals like ShadowRealm exist and can be used if they are ever implemented by all major browser vendors.

blocking actions from being forwarded for Special:Random would do nothing to deter a determined attacker from using other methods to achieve the same thing

Making things that only hackers do more expensive in terms of time is always good. Those obstacles add up.

I would put "call Special:Random?action=delete" in the bucket of "things that only hackers do", so I think I support this idea.

action=view is obviously useful. action=edit is probably someone's workflow. action=info probably isn't going to break anything either way. I can't think of a legitimate reason for the rest to work.

Thank you for tagging this task with good first task for Wikimedia newcomers!

Newcomers often may not be aware of things that may seem obvious to seasoned contributors, so please take a moment to reflect on how this task might look to somebody who has never contributed to Wikimedia projects.

A good first task is a self-contained, non-controversial task with a clear approach. It should be well-described with pointers to help a completely new contributor, for example it should clearly point to the codebase URL and provide clear steps to help a contributor get set up for success. We've included some guidelines at https://phabricator.wikimedia.org/tag/good_first_task/ !

Thank you for helping us drive new contributions to our projects <3

Special:Random with a specified action looks pretty uncommon. Here's a couple different wikitext searches checking to see if people use it:

The security incident in T419143: The wikis are currently read-only used Special:Random?action=edit and Special:Random?action=delete.

Poking around ActionEntryPoint.php and ActionFactory.php, it appears some of the common ?action=s in MediaWiki are:

  • credits
  • delete
  • edit
  • editchangetags
  • history
  • historysubmit
  • info
  • markpatrolled
  • mcrrestore
  • mcrundo
  • protect
  • purge
  • raw
  • render
  • revert
  • revisiondelete
  • rollback
  • submit
  • unprotect
  • unwatch
  • view
  • watch

Perhaps we block everything except view and edit, since these have no obvious user script impact. Perhaps with an error message For security reasons, action '$1' cannot be used on the page Special:Random. We can always block "edit" in a follow up patch if we decide to do so.

Change #1255946 had a related patch set uploaded (by Novem Linguae; author: Novem Linguae):

[mediawiki/core@master] SpecialRandomPage: disallow all actions except view and edit

https://gerrit.wikimedia.org/r/1255946

Catrope assigned this task to Novem_Linguae.
Catrope subscribed.

Thanks for the patch @Novem_Linguae!

Noting here for people who didn't see this on Gerrit that the patch that was merged also allows ?action=history, in addition to ?action=view (or no action) and ?action=edit.

Change #1255946 merged by jenkins-bot:

[mediawiki/core@master] SpecialRandomPage: disallow uncommon actions

https://gerrit.wikimedia.org/r/1255946

Thanks for the quick code review :)

If anyone feels strongly about blocking ?action=edit, please mention it here in the ticket and we can create a follow up ticket. For now I did not block it because I found 5 instances of it in global search, and there's no rush so we can make these changes incrementally. However there is certainly an argument to block it since it was used in the aforementioned security incident.