Custom JavaScript may yield privilege escalation
Closed, ResolvedPublic

Description

FINDING ID: iSEC-WMF1214-10

TARGETS: Users custom scripts, such as http://devwiki/w/index.php?title=User:Foo/common.js&action=
submit .

DESCRIPTION: When editing any user's custom JavaScript, the script is executed whenever the ``Show
Preview'' or ``Show Changes'' buttons are clicked. This could allow an attacker to trick another user
into executing JavaScript which hijacks their session. Because privileged users can edit the JavaScript
of lower privileged users, this could lead to privilege escalation.

EXPLOIT SCENARIO: A low-privileged user adds some complex custom JavaScript to their account,
with malicious code embedded within that directs people to a fake login screen or performs actions on
the victim's behalf. The user complains to an Administrator that they are having difficulty with their
custom ``skin'', and asks the Administrator to change a small portion of the script for them. Upon pre-
viewing the edit or viewing changes, the malicious code executes in the context of the Administrator's
session.

SHORT TERM SOLUTION: Do not include another user's custom script when previewing or showing
changes. These pages should only allow users to edit and view code.

LONG TERM SOLUTION: The custom JavaScript system has several deficiencies. Consider deprecating
this functionality and allowing users to customize the site using client-side code instead


Patches:

  • 1.24:
  • 1.23:
  • 1.19:

Affected Versions:
Type: XSS
CVE: CVE-2015-2938

csteipp created this task.Jan 5 2015, 8:52 PM
csteipp updated the task description. (Show Details)
csteipp raised the priority of this task from to Normal.
csteipp changed the visibility from "Public (No Login Required)" to "Security (Project)".
csteipp changed the edit policy from "All Users" to "Security (Project)".
csteipp changed Security from None to Software security bug.
csteipp added a subscriber: csteipp.
Restricted Application changed the visibility from "Security (Project)" to "Custom Policy". · View Herald TranscriptJan 5 2015, 8:52 PM
Restricted Application changed the edit policy from "Security (Project)" to "Custom Policy". · View Herald Transcript

I don't think this is very exploitable, but I think the short term solution is a good idea that we should implement.

Restricted Application changed the visibility from "Custom Policy" to "Custom Policy". · View Herald TranscriptJan 5 2015, 8:54 PM
Restricted Application changed the edit policy from "Custom Policy" to "Custom Policy". · View Herald Transcript
Anomie added a subscriber: Anomie.Jan 5 2015, 9:14 PM

LONG TERM SOLUTION: The custom JavaScript system has several deficiencies. Consider deprecating this functionality and allowing users to customize the site using client-side code instead

I don't think telling everyone to use Greasemonkey would go over all that well with "the community", if that's what they meant there.

Restricted Application changed the visibility from "Custom Policy" to "Custom Policy". · View Herald TranscriptJan 5 2015, 9:14 PM
Restricted Application changed the edit policy from "Custom Policy" to "Custom Policy". · View Herald Transcript
Anomie added a comment.Jan 5 2015, 9:34 PM

Krenair added a subscriber: Krenair.
csteipp claimed this task.Feb 9 2015, 10:20 PM

I'm wondering if this feature is useful for a site's common.js-- if admins on a site will use preview to test their changes before saving.

I think we should only prevent the preview if the title is also not in the project namespace.

Also, the "Tip: Use the "Show preview" button to test your new JavaScript before saving." message still shows up.

I'm wondering if this feature is useful for a site's common.js-- if admins on a site will use preview to test their changes before saving.

I think we should only prevent the preview if the title is also not in the project namespace.

This function is only called for previews of user CSS and JS subpages.

A feature request for previewing changes to site scripts might not be a bad idea, but since it doesn't seem to currently exist I don't think it should hold up this patch.

Also, the "Tip: Use the "Show preview" button to test your new JavaScript before saving." message still shows up.

New version to take care of that:

Thinking more about this, I'd like to get a product-y perspective on if/who this change might bother, so we can communicate in advance or find a better solution if it's going to affect too many people. @Tnegrin, do you know of anyone who might be good to review this?

Can @Tnegrin actually see this task? Phabricator shows them greyed out.

Krenair changed the visibility from "Custom Policy" to "Custom Policy".Feb 13 2015, 2:19 PM
Krenair changed the edit policy from "Custom Policy" to "Custom Policy".

Hi Chris, Krenair --

Let me ping the product and community liason teams. This seems like a good chance to understand who feels like they represent admins (who I'm thinking are the primary group who will be impacted by this change)

What's the timeframe on a decision here?

-Toby

Hi Chris, Krenair --

Let me ping the product and community liason teams. This seems like a good chance to understand who feels like they represent admins (who I'm thinking are the primary group who will be impacted by this change)

What's the timeframe on a decision here?

-Toby

Great! By the middle of next week would be nice.

csteipp changed the visibility from "Custom Policy" to "Custom Policy".Feb 14 2015, 12:24 AM
csteipp changed the edit policy from "Custom Policy" to "Custom Policy".
csteipp added a subscriber: Deskana.

I spoke with Chris about this. Here's my product opinion.

The circumstances in which this feature would actually be a security risk are rare. That said, the feature where previewing someone's JS actually executes it in your session is very obscure, and there are other, better ways that you can test JS that someone else is intended to use.

I think that he can remove this feature to fix this security problem.

New version to take care of that:

Tested. I'll deploy this early next week.

csteipp closed this task as Resolved.Feb 18 2015, 1:12 AM

deployed

bd808 moved this task from Done to Archive on the MediaWiki-Core-Team board.Feb 23 2015, 11:58 PM


csteipp updated the task description. (Show Details)Mar 27 2015, 10:03 PM
csteipp changed the visibility from "Custom Policy" to "Custom Policy".Mar 31 2015, 12:34 PM
csteipp changed the visibility from "Custom Policy" to "Public (No Login Required)".Mar 31 2015, 9:13 PM
csteipp changed the edit policy from "Custom Policy" to "All Users".
csteipp changed Security from Software security bug to None.

Change 201015 had a related patch set uploaded (by CSteipp):
SECURITY: Don't execute another user's CSS or JS on preview

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

Change 201026 had a related patch set uploaded (by CSteipp):
SECURITY: Don't execute another user's CSS or JS on preview

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

Change 201036 had a related patch set uploaded (by CSteipp):
SECURITY: Don't execute another user's CSS or JS on preview

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

Change 201015 merged by jenkins-bot:
SECURITY: Don't execute another user's CSS or JS on preview

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

Change 201026 merged by jenkins-bot:
SECURITY: Don't execute another user's CSS or JS on preview

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

Change 201036 merged by jenkins-bot:
SECURITY: Don't execute another user's CSS or JS on preview

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

He7d3r added a subscriber: He7d3r.Apr 1 2015, 12:47 AM

I assume the fix works for edit links with the URL parameter &preview=yes too?

I assume the fix works for edit links with the URL parameter &preview=yes too?

I don't think edit links with preview=yes previewed javascript before. But should not be allowed now in either case. If you're seeing different, please report it.

Change 201221 had a related patch set uploaded (by CSteipp):
SECURITY: Don't execute another user's CSS or JS on preview

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

Change 201221 merged by jenkins-bot:
SECURITY: Don't execute another user's CSS or JS on preview

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

csteipp updated the task description. (Show Details)Apr 9 2015, 11:11 PM
csteipp added a project: Vuln-XSS.
Verdy_p added a subscriber: Verdy_p.EditedApr 27 2015, 1:54 AM

Please don't follow the advice given in "LONG TERM SOLUTION". Custom javascripts are useful for many users that want it even if they are not supported as standard gadgets (gadgets are shared to be used by anyone without lot of inspection: users adopt them with a basic click in a checkbox and don't need to understand how they work, so only privileged users can transform a custom javascript or CSS into a plain gadget).
Custom javascript and CSS is also vbery useful for developing gadgets or experimenting changes.

The last solution given by gerribot above is enough to avoid executing another user's public CSS or JS just when editing, and submitting or previewing it (in any editor).


May be we'll also have custom Lua code in User's namespace (in that case, the same restriction will apply to avoid executing it, including when eding or viewing wiki pages #invoking these user's Lua extensions), instead of being stored in the "Module:" namespace (where we find now modules named "Module:User:XXXX/foo.lua" that pollute this shared namespace).

In that case, do not expand "{{#invoke:User:XXX/foo}}" and render it in "nowiki" or clear it completely, unless the viewing user is the same (but accept to render something like "{{#invoke:My user:foo}}" or within the profile of the current viewing user).

I'd like to note there are solutions for adding custom JS to sites outside of MediaWiki, e.g., Greasemonkey. Such solutions are safer and more performant than storing and loading such code to/from MediaWiki.

"safer"? Not really. There are good arguments for not allowing MediaWiki sites to reference foreign scripts located outside of sites that are not members of the same realm of wikis, because the wiki has absolutely no way to control how they are managed.
Also, MediaWiki is very efficient in loading javascripts using raw API requests across multiple wikis, and at least there's a possible control with improved security and a possible inspection by global admins: MediaWiki does not need to parse raw API requests, even if it has to perform a (single) database query to get their content.
But note that the issue discussed here is *only* about custom JS added to wiki sites that are running MediaWiki, and only MediaWiki.

Nothing is said in this security issue:

  • about how to load custom scripts on sites running other software than MediaWiki (such as blogs, or Facebook pages): these sites have their own way to permit such inclusion (or not) and define their own restrictions (or their own local security issues) ;
  • or about using external servers to retrieve custom JS to be used when viewing MediaWiki pages : this possibility could be restricted in MediaWiki by inspecting the target URL (notably the authorized domain names, or the interwiki prefixes if we can have external URLs built with them using some MediaWiki syntax for resolving wikilinks)

Also the "client-side" proposed "LONG TERM SOLUTION" is not portable across devices (notably because it requires preinstalling the code in them and copying it in multiple locations: a real maintenance problem for users that want to connect to Wiki using a guest access on a temporary device, including in cybercafés where users don't own their PC): this is impossible to do when using an external PC with resticted local installations.

And some devices don't have the necessary storage for allowing that: the "client" here being either a web browser, or a mobile wiki application for various mobile OSes (Android, iOS, Windows mobile... Additionally browsers won't let visited wiki sites to instruct them to load locally stored scripts (this would be a severe XSS issue in these browsers, if this is not restricted)

Please don't follow the advice given in "LONG TERM SOLUTION"

As anomie said above, I think we're all aware that doing that would go over like a pound of bricks. I'm highly doubtful we'd do that, and if we did there would be a lot of discussion about it. Thus lets save the pitch forks until somebody (not counting iSEC) actually makes a serious proposal to kill the feature.