Closed Bug 1599181 (CVE-2019-17016) Opened 1 year ago Closed 1 year ago

Bypass of CSS Sanitizer via incorrect serialization of CSS @namespace rule.

Categories

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

task

Tracking

()

VERIFIED FIXED
mozilla73
Tracking Status
firefox-esr68 72+ verified
firefox71 --- wontfix
firefox72 + verified
firefox73 + verified

People

(Reporter: michal.bentkowski, Assigned: emilio)

References

Details

(Keywords: csectype-disclosure, sec-moderate, Whiteboard: [reporter-external] [client-bounty-form] [post-critsmash-triage][adv-main72+][adv-esr68.4+])

Attachments

(7 files, 1 obsolete file)

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.

Flags: sec-bounty?

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....

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

Assignee: nobody → emilio
Group: firefox-core-security → core-security
Component: Security → CSS Parsing and Computation
Product: Firefox → Core
Summary: Bypass of CSS Sanitizer → Bypass of CSS Sanitizer via incorrect serialization of CSS @namespace rule.

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.

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.

Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(fbraun)

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.

Flags: needinfo?(fbraun) → needinfo?(dveditz)
Whiteboard: [reporter-external] [client-bounty-form] [verif?] → [reporter-external] [client-bounty-form]

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 - rtusp...@gmail.com is an actual email ;-)). This particular bug no longer works because of fix in Gmail itself.

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... :)

Group: core-security → layout-core-security
Flags: needinfo?(dveditz)
Keywords: sec-high

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.
Attachment #9111362 - Flags: sec-approval?

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

Approved to land and request uplift

Attachment #9111362 - Flags: sec-approval? → sec-approval+

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

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
Attachment #9111362 - Flags: approval-mozilla-esr68?
Attachment #9111362 - Flags: approval-mozilla-beta?
Group: layout-core-security → core-security-release
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla73

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

sec-high fix for 72.0b6 and 68.4

Attachment #9111362 - Flags: approval-mozilla-esr68?
Attachment #9111362 - Flags: approval-mozilla-esr68+
Attachment #9111362 - Flags: approval-mozilla-beta?
Attachment #9111362 - Flags: approval-mozilla-beta+

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.

Keywords: sec-moderate
Flags: qe-verify+
Whiteboard: [reporter-external] [client-bounty-form] → [reporter-external] [client-bounty-form] [post-critsmash-triage]
QA Whiteboard: [qa-triaged]

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.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Whiteboard: [reporter-external] [client-bounty-form] [post-critsmash-triage] → [reporter-external] [client-bounty-form] [post-critsmash-triage][adv-main72+]
Whiteboard: [reporter-external] [client-bounty-form] [post-critsmash-triage][adv-main72+] → [reporter-external] [client-bounty-form] [post-critsmash-triage][adv-main72+][adv-esr68.4+]

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

Flags: sec-bounty? → sec-bounty+
Keywords: sec-high
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.