Last modified: 2014-10-20 00:17:10 UTC
While investigating bug 68521 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?
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 [details] Proposed patch
(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 -------
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.
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 [details] 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.
Created attachment 16618 [details] 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.
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.
(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
(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.
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?
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 )
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.
