1599181 - (CVE-2019-17016) Bypass of CSS Sanitizer via incorrect serialization of CSS @namespace rule.

Status: VERIFIED FIXED

(Core :: CSS Parsing and Computation, task, P1)

Description

[Michał Bentkowski]

I've been recently researching the security of copy&paste in various browsers and noticed that Firefox perform CSS sanitization on <style> tags pasted from clipboard. The sanitizer could be abused in such a way that arbitrary CSS rules might be specified, allowing to data exfiltration attack.

In a nutshell, a <style> tag is pasted verbatim if the sanitizer decides that it doesn't contain any harmful content. Otherwise it rewrites the stylesheet, and gets rid of certain rules. Please follow the examples below to see how I came up with the exploit:

Example 1
INPUT: *{background:red}
SANITIZED OUTPUT: *{background:red}
COMMENT: The style is copied verbatim as it doesn't have any malicious rules.

Example 2:
INPUT: @import 'abc';*{background:red}
SANITIZED OUTPUT: * { background: red none repeat scroll 0% 0%; }
COMMENT: @import at-rule is disallowed, hence the CSS was rewritten.

Example 3
INPUT: @import'abc';@namespace x 'aa';{background:#eee}
SANITIZED OUTPUT: @namespace x url("aa"); { background: rgb(238, 238, 238) none repeat scroll 0% 0%; }
COMMENT: @namespace at-rule, on the other hand, is allowed. The URL of the namespace was rewritten from '...' syntax to url("...") syntax. At this point I had an idea that perhaps a quote character within the URL wouldn't be escaped when rewritten to the url("...") syntax.

Example 4
INPUT: @import'abc';@namespace x 'a"bc';{background:#eee}
SANITIZED OUTPUT: @namespace x url("a"bc"); { background: rgb(238, 238, 238) none repeat scroll 0% 0%; }
COMMENT: Please note the unescaped quote in the namespace URL. This means that arbitrary CSS rules can be injected - even @import!

Example 5
INPUT: @import'abc';@namespace x 'a"x);@import "data:text/css,{background:blue}";';
SANITIZED OUTPUT: @namespace x url("a"x);@import "data:text/css,{background:blue}";");
COMMENT: I'm injecting @import at-rule with data: URI to prove the vulnerability. At first sight it may appear as if the @import wouldn't execute since it follows @namespace which is disallowed. However, @namespace currently has an incorrect syntax (there's an "x" character right after closing quote), and is ignored by the CSS parser (as it was never there). @import is then treated as first valid rule of the CSS.

Example 6
INPUT: @import'abc';*{background: url('aaa"bbb')}
SANITIZED OUTPUT: * { background: rgba(0, 0, 0, 0) url("aaa"bbb") repeat scroll 0% 0%; }
COMMENT: Interestingly, the same problem doesn't happen in other contexts where rules are rewritten. This example shows that the URL of background is rewritten with proper escaping.

To summarize: because of improper rewriting of @namespace rule, it's possible to inject arbitrary CSS style.

PROOF OF CONCEPT:
I have prepared a PoC which exfiltrates CSS token from a hidden input using this very technique. The scenario is that the page contains a rich editor (i.e. a contenteditable element on the page) and the user just pastes the malicious content from clipboard. Because of some quirks in implementation of the attack, it requires HTTP/2, so it's first needed to create self-signed certificates:

1. Save css-exfiltration-firefox.js and vulnerable-page.html to some directory
2. openssl req -x509 -newkey rsa:2048 -nodes -sha256 -subj '/CN=localhost' -keyout localhost-privkey.pem -out localhost-cert.pem
3. npm install express spdy
4. node css-exfiltration-firefox.js
5. Open Firefox and navigate to both https://localhost:3000 and https://127.0.0.1:3000 and trust the certificate.
6. Go to https://localhost:3000 and press "Copy".
7. Go to vulnerable page and paste it in the contenteditable box.
8. See how the CSRF token is exfiltrated :)

I will also attach the video of the attack.

Comment 1

[Michał Bentkowski]

Attachment: Main file of data exfiltration exploit

Comment 2

[Michał Bentkowski]

Attachment: Example vulnerable page

Comment 3

[Michał Bentkowski]

Attachment: Screencast of the attack

Comment 4

[Frederik Braun [:freddy]]

Thanks for the detailed summary!

Emilio, is this something we need to fix in CSS parsing or later serialization? It seems to me we just want stuff within url("") to be quote-escaped? Probably need to think about hex/Unicode escapes too....

Comment 5

[Emilio Cobos Álvarez (:emilio)]

So I apparently noticed this when refactoring some code long time ago, and forgot to actually go and fix it, lol: https://searchfox.org/mozilla-central/rev/0678172d5b5c681061b904c776b668489e3355b0/servo/components/style/stylesheets/namespace_rule.rs#34

Comment 6

[Emilio Cobos Álvarez (:emilio)]

This is a stylo regression, and was introduced in https://hg.mozilla.org/mozilla-central/rev/2418cfba72c33c5623f6fb4c243c5203819c8240.

I looked at other callers of write_str() and they are ok.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Freddy / anyone else, mind giving a sec-rating here? Just to know whether I should check-in tests or we should wait until it's on release / etc.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1599181 - Fix serialization of @namespace rule. r=boris,#style

This code comes from:

https://hg.mozilla.org/mozilla-central/rev/2418cfba72c33c5623f6fb4c243c5203819c8240

I audited other callers of write_str, they seem ok.

Comment 9

[Frederik Braun [:freddy]]

I was inclined to call it sec-moderate until I saw the video in comment 3. Dan, would you be opposed to calling this sec-high?
I also want to note this well analyzed and in code we'd really like more eyes on.

Comment 10

[Michał Bentkowski]

Frederik asked me on Twitter to give my impact analysis on this bug. I feel that it is somewhere between moderate and high, here' s why:

• The attack might have significant impact on user security because it allows to leak data from certain websites.
• Website, to be vulnerable, might have a contenteditable field. It's pretty common in webmails, wikipedia etc.
• Some websites are protected against the attack because they implement onpaste event. Some of them just delete <style> tags from pasted content.
• Nevertheless, some high-profile websites have been vulnerable. I'm attaching a video of data exfiltration in Gmail (in the example I'm stealing email address of the user who is currently logged in; and yes - [e-mail] is an actual email ;-)). This particular bug no longer works because of fix in Gmail itself.

Comment 11

[Michał Bentkowski]

Attachment: Example of data exfiltration from Gmail

Comment 12

[Emilio Cobos Álvarez (:emilio)]

So it is pretty embarrassing that that code got in in the first place, but also that I noticed in 2017 while refactoring it (https://github.com/servo/servo/pull/17154) and forgot to fix it.

In fairness I was just a volunteer at the time, but... :)

Comment 13

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9111362 [details]
Bug 1599181 - Fix serialization of @namespace rule. r=boris,#style

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Relatively non-trivial, I guess.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
• Which older supported branches are affected by this flaw?: all
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: No
• If not, how different, hard to create, and risky will they be?: Should apply cleanly
• How likely is this patch to cause regressions; how much testing does it need?: not likely at all. It changes our serialization code to reuse more of our common serialization infrastructure.

Comment 14

[Tom Ritter [:tjr]]

Comment on attachment 9111362 [details]
Bug 1599181 - Fix serialization of @namespace rule. r=boris,#style

Approved to land and request uplift

Comment 15

[Emilio Cobos Álvarez (:emilio)]

Attachment: Minimal test-case for QA / etc purposes

I'll land proper automated tests when the bug goes public.

Comment 16

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9111362 [details]
Bug 1599181 - Fix serialization of @namespace rule. r=boris,#style

Beta/Release Uplift Approval Request

• User impact if declined: See comment 0
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce: I don't think it's needed, but the test-case above should not be red.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Minor serialization fix to make @namespace use the proper serialization primitives.
• String changes made/needed: none

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration:
• User impact if declined: see comment 0
• Fix Landed on Version:
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Minor serialization fix to make @namespace use the proper serialization primitives.
• String or UUID changes made by this patch: none

Comment 17

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/integration/autoland/rev/58f6b1b3bdd4ef1d62260ced3fdccf19856fc01d
https://hg.mozilla.org/mozilla-central/rev/58f6b1b3bdd4

Comment 18

[Julien Cristau [:jcristau]]

Comment on attachment 9111362 [details]
Bug 1599181 - Fix serialization of @namespace rule. r=boris,#style

sec-high fix for 72.0b6 and 68.4

Comment 19

[Daniel Veditz [:dveditz]]

I agree with the reporter that this is somewhere between a sec-moderate and sec-high. It's a sec-high impact on affected sites, mitigated by the need to get the victim to copy/paste from the attacker's content into a content-editable context. I guess we can tag it with both to make sure these fixes have visibility.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/0651a1910c6a

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr68/rev/38060b1ce782

Comment 22

[Daniel Bodea [:danibodea]]

I have reproduced this issue on Nightly v72.0a1 (2019-11-25) (64-bit) using the minimal test case uploaded in comment 15 (a red background blank page is displayed.
Furthermore, I have verified the fix in Nightly v73.0a1 (2019-12-22) (64-bit), Beta v72.0b7 and v72.0b9 and ESR68 v68.4.0esr (32-bit).
The reproduction and verification was performed on Windows 10 and Mac OS 10.15.

Comment 23

[Tom Ritter [:tjr]]

Attachment: advisory.txt

Comment 24

[Tom Ritter [:tjr]]

Attachment: advisory.txt

Comment 25

[Daniel Veditz [:dveditz]]

Although the effects may be similar to sec-high, getting there requires user interaction that limits this to sec-moderate.