Page MenuHomePhabricator

User specified CSS loads on Special:Preferences / Special:UserLogin
Closed, ResolvedPublic

Description

While investigating T70521 today, me and Helder noticed that user-created CSS is loaded on Special:Preferences and Special:UserLogin. Both special pages call $out->disallowUserJs(), which only controls TYPE_SCRIPTS, not TYPE_STYLES or TYPE_COMBINED.

Is this a bug? Or is it safe for CSS to run on those pages?


Version: unspecified
Severity: normal
See Also:

Details

Reference
bz70672

Event Timeline

bzimport raised the priority of this task from to Needs Triage.
bzimport set Reference to bz70672.
bzimport added a subscriber: Unknown Object (MLST).
Legoktm created this task.Sep 10 2014, 6:37 PM
He7d3r added a comment.EditedSep 10 2014, 6:46 PM

According to
https://bugzilla.wikimedia.org/show_bug.cgi?id=48931#c3
in IE in quirks mode you can also execute javascript using CSS:

.mw-special-Preferences {
 color:expression(importScript("..."));
}

(In reply to Helder from comment #1)

According to
https://bugzilla.wikimedia.org/show_bug.cgi?id=48931#c3
in IE in quirks mode you can also execute javascript using CSS:
.mw-special-Preferences {

color:expression(importScript("..."));

}

There are several ways to execute javascript from css. I would prefer we didn't allow it.

Aside from IE's non-standard way of executing code, there's also unlimited ways of tricking the user into performing actions by manipulating the interface from CSS (which then results in the unintended execution of otherwise trusted *server-side* code).

When ResourceLoader was given an "origin" marker, this was designed to apply to a module. Not to scripts only.

I believe disallowUserJs predates ResourceLoader and might just generally have an unfortunate name (legacy aside), and should imho disallow userCss as well.

Created attachment 16447
Proposed patch

Attached:

(In reply to Krinkle from comment #4)

Created attachment 16447 [details]
Proposed patch

Commit message

OutputPage: Remove separation of css and js module allowance

  • No longer segment module origin allowance by an "only=" content type. Both can be sensitive security-wise and there's no valid use case for allowing CSS anywhere you want to disallow JS. Both can significantly impact the user interface and cause unintended actions to be taken on the user's behalf, or desired actions to be made practically impossible.
  • While at it, also remove the ability to set the module allowance directly. The reduceAllowedModuleOrigin method is all we need. I couldn't find usage or mention of setAllowedModules() in mediawiki-core nor in any other Wikimedia-hosted repository.

Bug: 70672

Attached:

Tested, patch works fine so +1.

Nitpicks (not sure if these should be done as the original security patch or after it's public...):

  • We should deprecate disallowUserJs and create a disallowUserModules
  • OutputPage::filterModules and makeResourceLoaderLink shouldn't pass $type to getAllowedModules

The patch is working for me, and fixes the issue. I think lego's comments should be done in a public patch, to keep the security one small.

I'll get this deployed.

This bug not being public caused bug 71334 today.

greg added a comment.Sep 25 2014, 9:40 PM

Given the above breakage, we should probably get these patches out before the next monthly maintenance release.

Just to make sure: I assume this is deployed on the WMF servers already?

Created attachment 16617
Backport for REL1_23

MediaWiki 1.23 is still running... I found not exact way to reproduce the exploit, so I can't really say if the patch fixes the issue. I'd be extremely happy if someone else could have a look at the patch and confirm it works. Also, I have packported the deprecation notices. Not sure if that makes sense. If not, I will provide another patch without these notices. Please advise.

Attached:

Created attachment 16618
Backport to REL1_22

MediaWiki 1.22 is still running... I found not exact way to reproduce the exploit, so I can't really say if the patch fixes the issue. I'd be extremely happy if someone else could have a look at the patch and confirm it works. Also, I have packported the deprecation notices. Not sure if that makes sense. If not, I will provide another patch without these notices. Please advise.

Attached:

Created attachment 16619
Backport to REL1_19

MediaWiki 1.19 is still running... I found not exact way to reproduce the exploit, so I can't really say if the patch fixes the issue. I'd be extremely happy if someone else could have a look at the patch and confirm it works. Also, I have packported the deprecation notices. Not sure if that makes sense. If not, I will provide another patch without these notices. Please advise.

Attached:

(In reply to Markus Glaser from comment #10)

Just to make sure: I assume this is deployed on the WMF servers already?

Yes.

(In reply to Markus Glaser from comment #11)

MediaWiki 1.23 is still running... I found not exact way to reproduce the
exploit, so I can't really say if the patch fixes the issue. I'd be
extremely happy if someone else could have a look at the patch and confirm
it works. Also, I have packported the deprecation notices. Not sure if that
makes sense. If not, I will provide another patch without these notices.
Please advise.

To test it, add "body { background-color: red }" to your MediaWiki:Common.css. Note that your pages now have a red background. Visit Special:Preferences and/or Special:UserLogin. The red background should disappear if the patch is present and working.

Giving early access to Wikia

Grunny added a comment.Oct 1 2014, 2:26 PM

(In reply to Markus Glaser from comment #13)

Created attachment 16619 [details]
Backport to REL1_19

MediaWiki 1.19 is still running... I found not exact way to reproduce the
exploit, so I can't really say if the patch fixes the issue. I'd be
extremely happy if someone else could have a look at the patch and confirm
it works. Also, I have packported the deprecation notices. Not sure if that
makes sense. If not, I will provide another patch without these notices.
Please advise.

Tested 1.19 patch, and it's working fine.

Attached:

Tried Kunals test in all branches and it works.

Thanks Grunny for your help!!

Publishing this bug as the release is out.

Change 164271 had a related patch set uploaded by Legoktm:
SECURITY: OutputPage: Remove separation of css and js module allowance

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

Change 164271 merged by jenkins-bot:
SECURITY: OutputPage: Remove separation of css and js module allowance

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

(In reply to Markus Glaser from comment #15)

Giving early access to Wikia

What about those of us who are running giant platforms as well such as Gamepedia?

Alexia added a comment.Oct 3 2014, 2:42 PM

It appears this change is also affecting custom skin CSS(Mediawiki:Vector.css) instead of only cusotm user CSS. This prevents custom site styles, that are only editable by the site administrators, loaded through Mediawiki:Vector.css to not display when on those pages. Unfortunately that ends up being a jarring experience for the end user.

FWIW, I don't think the security benefit (which is at best minimal) of this change is worth the inconvenience to users who do custom skinning by editing MediaWiki:Common.css (See also my email to wikitech-l https://lists.wikimedia.org/pipermail/wikitech-l/2014-October/078903.html )

Kghbln added a comment.Oct 7 2014, 7:49 AM

This is indeed a big problem for wikis which use on-wiki custom skinning. Besides this indeed rather the regular case than a rare one. Now with some days having passed I can report that people even thought their login was maliciously hijacked since this page now looks totally different than the rest of the wiki. While the actual security increased the felt security dramatically plunged. :( I utterly agree with Bawolff.

I'm in agreement with Alexia and Bawolff (and kgh). For sites that use that use massive custom changes to their skins, such as the background color or the sidebar, having all this not show up on Special:Preferences or Special:UserLogin really takes away more than the minimal security it adds. Given that, as Alexia said, only administrators can even edit these interface pages, it's only reasonable that they should affect the entire site. Using my own site as example: http://grisaiawiki.net/ where I changed all sorts of colors and styles through Common.css, I was a bit off-put when I noticed that my changes aren't showing up on a few pages. User-specific CSS and JS not showing up on these pages is fair, but site-wide interface edits should get through.

I talked with Kunal about this yesterday. My perspective is that admin controlled css is probably the least likely place someone is going to inject something malicious. The user controlled css is the part that scares me the most.

I'd be ok with a config option to allow Common.css and the skin css files through. I'm not sure how much work that would be in resource loader.

Bug 71621 is tracking the issue of site-wide styles not being loaded, and I've uploaded a patch for it.

He7d3r updated the task description. (Show Details)Mar 16 2015, 11:20 AM
He7d3r set Security to None.

When will this be fixed? I installed MW 1.27 and Special:Preferences is still broken.

Restricted Application added a subscriber: Malyacko. · View Herald TranscriptJul 4 2016, 11:45 AM

@Subfader You will have to set the $wgAllowSiteCSSOnRestrictedPages configuration parameter to "true".