Closed Bug 1285003 (CVE-2016-9071) Opened 8 years ago Closed 8 years ago

Probe browser history via HSTS/301 redirect + CSP


(Core :: DOM: Security, defect)

47 Branch
Not set



Tracking Status
firefox49 --- wontfix
firefox50 --- fixed


(Reporter: xiaoyin.l, Assigned: ckerschb)



(Keywords: csectype-disclosure, privacy, sec-low, Whiteboard: [post-critsmash-triage][domsecurity-active][adv-main50+] Embargo until chrome fixed)


(3 files, 1 obsolete file)

Attached file csp probing.html
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/46.0.2486.0 Safari/537.36 Edge/13.10586

Expected results:

This is an updated version of the Sniffly attack (for the original attack, see The original Sniffly vulnerability was fixed by making the CSP directive "img-src http:" === "img-src http: https:". But I find a way to bypass this fix, and make it possible again to probe which domains or URLs have been visited by users: the new trick is to use "Content-Security-Policy: img-src". It allows, but blocks This means by setting this CSP directive, I can now measure the time it takes for Firefox to redirect from http to https, and see if it happens due to a cache or a server-side redirect, which can be used to guess whether the user has visited that link before.

This vulnerability is reproducible on Firefox 47.0.1 (stable version, Windows 10 x64).

The PoC is attached. To reproduce: 1) clear Firefox cache; 2) disable HTTPS Everywhere if you have it installed; 3) visit; 4) visit; 5) open the PoC in the same Firefox window.

Please note that this bug is also present in Chrome (tracked on So please don't disclose this bug until Chromium fixes or rejects this issue on their side. Thanks!
Group: firefox-core-security → core-security
Component: Untriaged → DOM: Security
Product: Firefox → Core
Ever confirmed: true
Group: core-security → dom-core-security
Whiteboard: [domsecurity-backlog]
Assignee: nobody → ckerschb
Whiteboard: [domsecurity-backlog] → [domsecurity-active]
Dan, for quite some time I wanted to slightly refactor our port matching algorithm, I think now is a good time to do that and also incorporate the latest updates for port matching.
Attachment #8772146 - Flags: review?(dveditz)
Whiteboard: [domsecurity-active] → [domsecurity-active] Embargo until chrome fixed
Google has fixed this issue in Chrome 52. They assigned it CVE-2016-5137. WebKit is still working on a fix.

Comment on attachment 8772146 [details] [diff] [review]

Review of attachment 8772146 [details] [diff] [review]:

My comments are mostly style (and a little about perf) which I leave up to you. r=dveditz

::: dom/security/nsCSPUtils.cpp
@@ +469,5 @@
> +  resourcePort = (resourcePort > 0)
> +                 ? resourcePort
> +                 : NS_GetDefaultPort(resourceScheme.get());
> +
> +  // If ports match, allow the load.

or at least "don't block the load" -- other things checked elsewhere are necessary to allow the load.

@@ +472,5 @@
> +
> +  // If ports match, allow the load.
> +  nsString resourcePortStr;
> +  resourcePortStr.AppendInt(resourcePort);
> +  if (aEnforcementPort.Equals(resourcePortStr)) {

This is going to fail most of the time: ports will be the default, but you've turned resource port into an explicit "443" or "80", and the enforcement port will also be empty. Might be worth avoiding the string creation/manipulation with a special-case default-port check like

   if (resourcePort == -1 && aEnforcementPort.IsEmpty()) {
      return true;

(use a "#define DEFAULT_PORT -1" or something for readability). Obviously this depends on scheme checks elsewhere!

If you get past that you know you have at least one explicit port, so now you can do the getDefaultPort()/resourcePortStr.AppendInt() bit.

@@ +502,5 @@
> +  httpDefaultPortStr.AppendInt(NS_GetDefaultPort("http"));
> +  httpsDefaultPortStr.AppendInt(NS_GetDefaultPort("https"));
> +
> +  if (enforcementPort.Equals(httpDefaultPortStr) &&
> +      resourcePortStr.Equals(httpsDefaultPortStr)) {

seems like extra unnecessary work when the strings are effectively constants. Why not just

  if (enforcementPort.EqualsLiteral("80") &&
      resourcePortStr.EqualsLiteral("443")) {

@@ +608,5 @@
>      }
>    }
> +  // Port matching: Check if the ports match.
> +  if (!permitsPort(mScheme, mPort, aUri)) {

Shouldn't we check the port before checking path? In the end everything needs to match, but logically (and in the spec) we should validate the origin first. Now that this bit is so small (rather than a big inline chunk) it's easy to move.
Attachment #8772146 - Flags: review?(dveditz) → review+
Attachment #8772147 - Flags: review?(dveditz) → review+
Thanks Dan, incorporated your optimizations and nits.
Carrying over r+.
Attachment #8772146 - Attachment is obsolete: true
Attachment #8773660 - Flags: review+
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Group: dom-core-security → core-security-release
Flags: qe-verify-
Whiteboard: [domsecurity-active] Embargo until chrome fixed → [post-critsmash-triage][domsecurity-active] Embargo until chrome fixed
Any updates on webkit fixing this?
Flags: needinfo?(xiaoyin.l)
Whiteboard: [post-critsmash-triage][domsecurity-active] Embargo until chrome fixed → [post-critsmash-triage][domsecurity-active][adv-main50+] Embargo until chrome fixed
(In reply to Al Billings [:abillings] from comment #8)
> Any updates on webkit fixing this?

None yet. There have been no replies from WebKit after they confirmed the issue. I just left another comment there asking for updates.
Alias: CVE-2016-9071
Group: core-security-release
(In reply to Al Billings [:abillings] from comment #8)
> Any updates on webkit fixing this?

It was released as part of Safari Technology Preview Release 21.
Flags: needinfo?(xiaoyin.l)
You need to log in before you can comment on or make changes to this bug.