1285003 - (CVE-2016-9071) Probe browser history via HSTS/301 redirect + CSP

Status: RESOLVED FIXED

(Core :: DOM: Security, defect)

Description

[Xiaoyin Liu]

Attachment: 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 https://bugs.chromium.org/p/chromium/issues/detail?id=544765). 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 http://example.com:80". It allows http://example.com, but blocks https://example.com. 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 http://www.mozilla.org/; 4) visit http://www.npmjs.com/; 5) open the PoC in the same Firefox window.

Please note that this bug is also present in Chrome (tracked on https://bugs.chromium.org/p/chromium/issues/detail?id=625945). So please don't disclose this bug until Chromium fixes or rejects this issue on their side. Thanks!

Comment 1

[Christoph Kerschbaumer [:ckerschb]]

Attachment: bug_1285003_port_80_should_match_443.patch

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.

Comment 2

[Christoph Kerschbaumer [:ckerschb]]

Attachment: bug_1285003_port_80_should_match_443_tests.patch

Comment 3

[Xiaoyin Liu]

Google has fixed this issue in Chrome 52. They assigned it CVE-2016-5137. WebKit is still working on a fix.

[1] https://googlechromereleases.blogspot.com/2016/07/stable-channel-update.html
[2] https://bugs.webkit.org/show_bug.cgi?id=159520

Comment 4

[Daniel Veditz [:dveditz]]

Comment on attachment 8772146 [details] [diff] [review]
bug_1285003_port_80_should_match_443.patch

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.

Comment 5

[Christoph Kerschbaumer [:ckerschb]]

Attachment: bug_1285003_port_80_should_match_443.patch

Thanks Dan, incorporated your optimizations and nits.
Carrying over r+.

Comment 6

[Christoph Kerschbaumer [:ckerschb]]

I think I should add those myself since it's a hidden bug:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bbfdbbf1dab4
https://hg.mozilla.org/integration/mozilla-inbound/rev/fbaa8f469f5a

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/bbfdbbf1dab4
https://hg.mozilla.org/mozilla-central/rev/fbaa8f469f5a

Comment 8

[Al Billings [:abillings - ex-MoCo]]

Any updates on webkit fixing this?

Comment 9

[Xiaoyin Liu]

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

Comment 10

[Xiaoyin Liu]

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