Page MenuHomePhabricator

CSS using var() to create exponential sized calc() on wiki page will crash visitor's browser
Closed, ResolvedPublic

Description

Summary:
It is possible to place malicious code on any wiki-page in any project of the Wikimedia Foundation (Wikipedia, Commons and others). It consists of a HTML element with an inline CSS style, which contains CSS variables. By abuse of the variable evaluation mechanism it is possible to cause a crash of all modern browsers.
A working sample of the code is attached to this ticket (see the file attack_code_example.txt).

I’m an administrator and CU on the Russian Wikipedia and have found this sample in the list of recent edits today! After it crashed my browser a couple of times I was able to retrieve an inactive sample by deleting the affected page and viewing the source code during undeleting.

Possible impact:
High to critical, as it will crash browsers of page visitors, especially if injected into a widely-used template. In case the code manages to reach the main page via inclusions – it will greatly affect all users and visitors of the project. Such malicious edits could be quite difficult to undo, as the diff-view will also cause a crash. And the most worrying thing about the issue - I caught the code "in the wild"...

How to reproduce:

  1. Copy the sample code from the attached text file (attack_code_example.txt) and paste it to any page (article, discussion page, user page, new page, existing page, etc.)
  2. Press “Show preview”. The browser will crash immediately.
  3. If you publish the page instead of previewing (DON’T DO IT), the embedded code will crash the browser of every visitor of the infected page.

Tested configurations:

  • Browsers affected: Chrome 70.0, Firefox 63
  • Browsers not affected: IE11.
  • Sites affected: All WMF sites - tested in RU Wiki, EN Wiki, DE Wiki and on Commons. Also, numerous external wiki-projects, based on MediaWiki engine.

Possible remedies:

  • Immediate: New abuse filter rule - already did it on RU Wiki
  • Long term: Deactivate interpretation of CSS variables (the same approach as with JS snippets on general pages).

Event Timeline

Please also hide it from the general public!

Q-bit-array triaged this task as Unbreak Now! priority.Nov 6 2018, 7:48 PM

Patch looks good to me, but this certainly seems like something we should only deploy temporarily, until browser vendors get it together and make their own fixes to stop the browsers from crashing. Hopefully we're not the first people to discover this problem, but I don't know if we have anyone with access to security bugs in the browsers' bug trackers, who could see if it's already reported.

Disallowing calc() and var() in wikitext forever would be a bit overkill, they are otherwise nice and useful features. Currently we only disallow CSS syntax that allows loading external images etc. (which is a privacy problem).

Actually, I would suggest changing the patch to only disallow var(). By itself, calc() can not cause this issue, and it is likely to have been used legitimately in articles, which would now break (and cause people to ask questions about why we're disallowing it).

but I don't know if we have anyone with access to security bugs in the browsers' bug trackers, who could see if it's already reported.

[Royal] we can always file duplicates at https://bugzilla.mozilla.org/enter_bug.cgi?product=Firefox and https://bugs.chromium.org/p/chromium/issues/ ?
Would be the same situation for anyone who wants to report a WM security bug and isn't in Security... :)

until browser vendors get it together and make their own fixes to stop the browsers from crashing

Don't think the browser vendors will fix the issue soon. I was able to reproduce the crash even in an old Firefox 52 LTS...

If calc() is safe (and this would require an investigation to confirm), site CSS and TemplateStyles can use them, but honestly, this is just too shenanigany thing to allow in, and very close to expression() that we've been blacklisting for ages.

var() is pretty useless in inline styles, i think it makes sense to ban it permenently there.

I think its already banned in templatestyles but not sure

just blacklist var() but keep calc() allowed

btw, it seems like a more minimal test case (at least on chrome) is:

	<div style="--a:1px;--b:calc(var(--a) + var(--a));--c:calc(var(--b) + var(--b));--d:calc(var(--c) + var(--c));--e:calc(var(--d) + var(--d));--f:calc(var(--e) + var(--e));--g:calc(var(--f) + var(--f));--h:calc(var(--g) + var(--g));--i:calc(var(--h) + var(--h));--j:calc(var(--i) + var(--i));--k:calc(var(--j) + var(--j));--l:calc(var(--k) + var(--k));--m:calc(var(--l) + var(--l));--n:calc(var(--m) + var(--m));--o:calc(var(--n) + var(--n));--p:calc(var(--o) + var(--o));--q:calc(var(--p) + var(--p));--r:calc(var(--q) + var(--q));--s:calc(var(--r) + var(--r));--t:calc(var(--s) + var(--s));--u:calc(var(--t) + var(--t));--v:calc(var(--u) + var(--u));--w:calc(var(--v) + var(--v));--x:calc(var(--w) + var(--w));--y:calc(var(--x) + var(--x));">Crash</div>

e.g. You don't have to actually apply the resulting property anywhere, just defining the css variables with the exponential explosion of variable references, seems to be enough.

Bawolff renamed this task from Malicious code on a wiki page will crash visitor's browser to CSS using var() to create exponential sized calc() on wiki page will crash visitor's browser.Nov 8 2018, 5:42 AM

[05:30] bawolff !log deployed patch T208881

With this patch, we no longer allow var() in inline styles, which should prevent this sort of exponential css properties.

So I assume this is basically an OOM error due to exponential expansion (I haven't actually verified that explicitly, but it stands to reason). And at first glance there is no reason to suspect that this has security relavence other than as a denial of service attack. According to https://www.chromium.org/Home/chromium-security/reporting-security-bugs Chrome doesn't consider this to be a "security" issue, and we should probably report this normally.

I'm not sure about firefox, but I think they would view it similarly.

Before I publicly do that, I want to double check, does that sound correct?

Bawolff lowered the priority of this task from Unbreak Now! to Medium.Nov 8 2018, 6:16 AM

[reducing priority now that our sites are patched]

So I assume this is basically an OOM error due to exponential expansion (I haven't actually verified that explicitly, but it stands to reason). And at first glance there is no reason to suspect that this has security relavence other than as a denial of service attack. According to https://www.chromium.org/Home/chromium-security/reporting-security-bugs Chrome doesn't consider this to be a "security" issue, and we should probably report this normally.

I'm not sure about firefox, but I think they would view it similarly.

Before I publicly do that, I want to double check, does that sound correct?

There's the further issue, of if indeed upstream would consider this a public bug, then we'd probably want to do the next MW release before reporting it (I guess)?

I think its already banned in templatestyles but not sure

It's currently not whitelisted in TemplateStyles, mainly because there seems to be no way to effectively sanitize rules using it.

There's some relevant discussion in T200632#4623299.

There is now a fancy name for this vulnerability, apparently. The page mentions experiments with Wikipedia and MediaWiki.

https://cras.sh/

Breaking the UI on pages with public write access that allow some HTML tags with inline styles. Wikipedia(example, crashes the browser they've banned the account for vandalism, even though I've placed it on the personal page) and most MediaWiki-based projects are affected. Basically, one can break the page and it wouldn't be reparable through UI.

https://www.reddit.com/r/programming/comments/a16jn6/crassh_crossbrowser_purely_jsless_way_to_crash/

With everything going on, i forgot to file an upstream bug. We should do that

With everything going on, i forgot to file an upstream bug. We should do that

Did this get done? ;)

Reedy assigned this task to Bawolff.
Reedy changed the visibility from "Custom Policy" to "Public (No Login Required)".Jun 6 2019, 4:01 PM

Change 514763 had a related patch set uploaded (by Reedy; owner: MaxSem):
[mediawiki/core@REL1_27] SECURITY: blacklist CSS var()

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

Change 514763 merged by Reedy:
[mediawiki/core@REL1_27] SECURITY: blacklist CSS var()

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

Change 514774 had a related patch set uploaded (by Reedy; owner: MaxSem):
[mediawiki/core@REL1_30] SECURITY: blacklist CSS var()

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

Change 514774 merged by Reedy:
[mediawiki/core@REL1_30] SECURITY: blacklist CSS var()

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

Change 514754 merged by jenkins-bot:
[mediawiki/core@master] SECURITY: blacklist CSS var()

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

Change 514850 had a related patch set uploaded (by Reedy; owner: MaxSem):
[mediawiki/core@REL1_31] SECURITY: blacklist CSS var()

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

Change 514850 merged by jenkins-bot:
[mediawiki/core@REL1_31] SECURITY: blacklist CSS var()

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

Change 514950 had a related patch set uploaded (by Reedy; owner: MaxSem):
[mediawiki/core@REL1_32] SECURITY: blacklist CSS var()

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

Change 514950 merged by jenkins-bot:
[mediawiki/core@REL1_32] SECURITY: blacklist CSS var()

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

Change 514974 had a related patch set uploaded (by Reedy; owner: MaxSem):
[mediawiki/core@REL1_33] SECURITY: blacklist CSS var()

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

Change 514974 merged by jenkins-bot:
[mediawiki/core@REL1_33] SECURITY: blacklist CSS var()

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

Locally on Gamepedia we are removing this patch due to it negativity affecting many of our wikis breaking styling and layout. This issue only affects Chrome the worst, does not crash Safari(though never finishes loading), and loads fine in Firefox. It should be fixed by the browser vendors as needed.

It works fine on Firefox because they fixed it (https://bugzilla.mozilla.org/show_bug.cgi?id=1510862), so its just Chrome, which is still a lot of users. Also, I believe the patch was only intended to be done temporarily until browsers patched it, which Chrome still hasn't done.

Hmm. it appears that chrome already patched this