Closed Bug 767778 (CVE-2012-1963) Opened 12 years ago Closed 12 years ago

Content Security Policy: violation reports leak OAuth 2.0 and OpenID credentials

Categories

(Core :: Security, defect)

defect
Not set
major

Tracking

()

RESOLVED FIXED
Tracking Status
firefox14 + fixed
firefox15 + fixed
firefox16 + fixed
firefox-esr10 14+ fixed

People

(Reporter: karthikeyan.bhargavan, Assigned: dveditz)

References

Details

(Keywords: sec-high, Whiteboard: [advisory-tracking+][qa?])

Attachments

(3 files)

The Content Security Policy (CSP) violation reports generated by Firefox include sensitive data within the "blocked-uri" parameter. In particular, they include fragment components and query strings even if the blocked-uri has a different origin than the protected resource (violating CSP 1.1 recommendations). Hence, a malicious website may use Firefox's CSP implementation to steal a user's OAuth 2.0 access tokens and OpenID credentials. This leads to serious attacks on popular social authorization servers such as Facebook and Google.

For example, suppose a Yahoo user has previously used "Sign in with Facebook" to log in to Yahoo. 
Suppose also that the user is currently logged in to Facebook (she does not have to be logged in to Yahoo.) 
If she then visits attacker.com, this website can redirect her to Facebook's OAuth 2.0 endpoint, requesting an access token for the current user at Yahoo.
Facebook issues a token and forwards the user to open.login.yahoo.com/#access_token=AAAXXX
The key security mechanism of OAuth 2.0 is that this access token will only be sent to Yahoo, so even if the original token request was sent by an attacker, he will not be able to obtain the access token.

Now, suppose attacker.com had a CSP that forbid connections to open.login.yahoo.com.
Then Firefox will send a CSP violation report to "report-uri" that contains as its "blocked-uri" the full URL open.login.yahoo.com/#access_token=AAAXXX
Hence, the attacker obtains the user's Facebook access token at Yahoo and may download sensitive user information from the Facebook API.
For comparison, Chrome sends a "blocked-uri" that contains only the domain open.login.yahoo.com, avoiding the attack.

A similar attack works for the Facebook OAuth 2.0 authorization code flow, allowing the attacker to steal the auth code and potentially log in as the user on Yahoo.

A similar attack works on the Google OpenID interaction, where the attacker obtains the user's name and email address from Google and also a signed OpenID response that the attacker may potentially use to log in as the user to Yahoo.

Demo:
----

As a proof of concept, we have built an attack webpage that silently retrieves (and stores) any visitor's names and email addresses from Facebook and Google.
The preconditions for this demonstration: the visitor must be logged into Facebook or Google (in the same browser) and must have previously authorized Yahoo to access his Facebook or Google profile (by using Sign in with Facebook for example).
The demo is at http://moscova.inria.fr/~karthik/csp/index.html

Causes and Fixes:
-----------------

The attack shown here was predicted (for a fictionalized version of OAuth) by Adam Barth in http://lists.w3.org/Archives/Public/public-web-security/2011Apr/0055.html
Our demo shows that it is an effective attack on real OAuth and OpenID users,
such as those using the Google, Facebook, Live, LinkedIn, and Dropbox APIs.

The root issue is that Firefox does not implement the recommendations of CSP 1.1 on violation reports (section 4.15). Implementing them would disable the attack above. 

Specifically, Firefox must remove the <fragment> component from the blocked URI. This would prevent Facebook access tokens from being leaked.

Since "the origin of the blocked-uri is not the same as the origin of the protected resource", Firefox should also removes the query string. This would prevent the Google OpenID identifiers from being leaked.

About Us:
--------
I lead a security research group called Prosecco at INRIA in France.
http://prosecco.gforge.inria.fr
Our work focuses on formal security proofs for cryptographic and web applications. We found this attack as part of a formal study of OAuth
http://www.doc.ic.ac.uk/~maffeis/csf12.pdf
We would welcome the opportunity to study any fixes you may propose and incorporate them into our analyses.
Product: Firefox → Core
QA Contact: firefox → toolkit
Confirmed. Technically a duplicate of bug 604177 but I'll leave this one open since it has more explanation. Originally the Mozilla implimentation required that reports be sent only to the original host because of this kind of concern. We've relaxed that to a somewhat bogus "same base domain" (based on earlier spec discussions) and are not yet spec-compliant in allowing any report-uri domain.

Content Security Policy 1.0 isn't official yet (in Last Call, though) so it's better to cite that standard than 1.1 which has received no discussion. The current "1.1" spec is simply Adam's first pass at the various proposed additions whether there's consensus or not. The reporting bit is the same so that doesn't invalidate this bug in any way.

I'm not convinced the spec's solution is a whole lot better. OAuth 2.0 might use the fragment but other services might use a query parameter or some other part of the URL. We really ought to just report the original URL--even if that URL itself passes the policy--and simply add a notation that it redirects. Maybe the redirect target hostname would be safe enough.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Updating references to the draft: in CSP 1.0, the relevant section is 4.11 

The recommendation over there that avoids the attack is "If the origin of the blocked-uri is not the same as the document's origin, then replace the blocked-uri with the ASCII serialization of the blocked-uri's origin." 

I interpret this as: "blocked-uri" should not contain anything other than the scheme/host/port of the redirected URI; in the example above just "https://open.login.yahoo.com". Is that correct?
CC'ing abarth for interpretation of the spec (see comment 2). 

I think you're looking at an old version of the spec since the relevant section is 3.2.11 -- please see the latest at http://dvcs.w3.org/hg/content-security-policy/raw-file/tip/csp-1.0-specification.html -- but the language is the same. I'd interpret that they way you did, except the (non-normative) example in 3.3.2 gives the document-uri as http://example.org/page.html and the full blocked-uri of http://evil.example.com/image.png is sent even though they are not the same origin.

In the scenario you posit the attackers page knows the URIs present in the page itself so the example violation report would not tell it anything it didn't already know, it's only the redirect case that's dangerous. In the case of a legit page with an injected policy then even same-origin blocked-uris might leak information.
I see the current spec now, thanks. I agree that the redirect case is the one that is most dangerous, specifically when the new URI contains a secret that is not meant for the CSP-protected page. 

Using origins to decide the scope of the secret is probably too crude (it is easy to find cases where one page on a website is not meant to see a query parameter sent to another.) 

However, revealing only the original resource URI and not the redirection URI seems safe. I believe this corresponds to existing policies on following redirections in iframe.src: outside the iframe only the original src URI is available, although inside the iframe the final URI is known.
Assigning to Tanvi because she's been going to the spec meetings, but pass it along if there's someone more free.
bug reporter requested via email to sec@ that we may want to talk to our contacts at google and facebook about this
Tanvi seems swamped, I'll take a crack.
Assignee: tanvi → dveditz
This patch implements the latest draft of the 1.0 spec recommendation to strip the fragment from both document-uri and blocked-uri, and further truncates blocked-uri to just the origin (scheme-host-port) for redirects. This differs slightly from the spec in that the spec says blocked-uri should be so truncated for any request whose origin does not match the document-uri. The spec's behavior would truncate useful information that the page could already find by crawling the DOM.

Other minor changes not strictly related to the security fix but worth addressing while we're messing with the report so CSP-using sites don't have to adjust too many times:
 - the report "request" was replaced with "document-uri" per spec
 - "referrer" was added per spec
 - original-uri was added in the redirect case, to help site
   defenders figure out where mysterious blocked origins came from.
   the original-uri is what the site had in the document to start with
   so it shouldn't raise any privacy concerns, or it was injected by
   attackers so it's exactly what the defenders want to know.

Tagging Sid for the official code review, Tanvi for optional feedback since she's been involved with the WG spec discussions (feel free to remove yourself if you don't have any).
Attachment #640133 - Flags: review?(sstamm)
Attachment #640133 - Flags: feedback?(tanvi)
Comment on attachment 640133 [details] [diff] [review]
Only show origin for redirects

Review of attachment 640133 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with the tweaks below (most of the changes just tweak the contents of the violation report and are not risky).  Wish there were tests, but they wouldn't be high-value since most of this change is in one line.

::: content/base/src/contentSecurityPolicy.js
@@ +194,5 @@
>      this._docRequest = aChannel;
>  
> +    // save the document URI (minus <fragment>) and referrer for reporting
> +    this._request = aChannel.URI.cloneIgnoringRef().asciiSpec;
> +    this._referrer = aChannel.referrer.cloneIgnoringRef().asciiSpec;

Probably want to nullify userpass here too.

@@ -205,5 @@
> -      var reqMaj = {};
> -      var reqMin = {};
> -      var reqVersion = internalChannel.getRequestVersion(reqMaj, reqMin);
> -      this._request += " HTTP/" + reqMaj.value + "." + reqMin.value;
> -    }

Just to confirm, we're taking the request method out of the report?  So the recipient won't know if the page was a GET or POST result?

@@ +256,5 @@
> +          blocked += "//"+blockedUri.asciiHost;
> +          if (blockedUri.port != -1) {
> +            blocked += ":"+port;
> +          }
> +        }

Re-constructing a URI as a string worries me a little bit here.  Is there another way to do this, such as via cloning, setting the path and userpass to be null, then generating asciiSpec?

::: content/base/src/nsCSPService.cpp
@@ +95,5 @@
>                      ("Document has CSP: %s",
>                       NS_ConvertUTF16toUTF8(policy).get()));
>  #endif
>              // obtain the enforcement decision
> +            // (don't pass aExtra, we use that slot for redirects)

While I don't like the idea of shoehorning this into aExtra, I think it's fine for now.  Really we should call this function something other than ShouldLoad, and only pass along the minimal set of things.  That's for another bug though.
Attachment #640133 - Flags: review?(sstamm) → review+
Comment on attachment 640133 [details] [diff] [review]
Only show origin for redirects

[Triage Comment]
Dan and I just discussed this bug. It seems like a significant enough security issue, low risk, and has very little chance for fallout. Let's land on all branches in preparation for Firefox 14's release.
Attachment #640133 - Flags: approval-mozilla-esr10+
Attachment #640133 - Flags: approval-mozilla-beta+
Attachment #640133 - Flags: approval-mozilla-aurora+
Minor update to address Sid's review comments.

> Probably want to nullify userpass here too.

Done.

> Just to confirm, we're taking the request method out of the report?  So
> the recipient won't know if the page was a GET or POST result?

The spec currently has the plain document-uri amd we need to support that to help web sites have sane log-parsing software that works across browsers. If you think the method is important we could add an additional property to the JSON blob.

> Re-constructing a URI as a string worries me a little bit here.

Made me feel a little dirty, too. Changed as suggested.
Attachment #640528 - Flags: review+
Attachment #640528 - Flags: approval-mozilla-esr10+
Attachment #640528 - Flags: approval-mozilla-beta+
Attachment #640528 - Flags: approval-mozilla-aurora+
Backed out of mozilla-central and aurora for xpcshell failures. I couldn't see you on IRC, were you still watching the tree after landing?

https://tbpl.mozilla.org/?rev=1cefe3f794ba
https://tbpl.mozilla.org/?tree=Mozilla-Aurora&rev=1bf28b3b5464

https://hg.mozilla.org/mozilla-central/rev/0b4a966d3f85
https://hg.mozilla.org/releases/mozilla-aurora/rev/7a8720059286
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla16 → ---
Fixes the xpcshell test failure by catching the exception (which is harmless in this context, but ugly)
Attachment #640662 - Flags: review?(sstamm)
Comment on attachment 640662 [details] [diff] [review]
fix test failures

Review of attachment 640662 [details] [diff] [review]:
-----------------------------------------------------------------

Yeah, do this.
Attachment #640662 - Flags: review?(sstamm) → review+
Whiteboard: [advisory-tracking+]
I'm not sure I know what I'm doing here but access tokens appear to be visible using the demo site in comment 0 with Firefox 10.0.4esr (expected) and Firefox 10.0.6esrpre 2012-07-11 (unexpected). Can someone provide simplified instructions so I can verify this? Here are the steps I was following:

1. Log in to Facebook.com
2. Log in to Yahoo.com in another tab using "sign in with Facebook"
3. Open http://moscova.inria.fr/~karthik/csp/index.html in a third tab
4. Click "Farmed Access Tokens" link
Result: I see access tokens with my username from Yahoo and Facebook
Whiteboard: [advisory-tracking+] → [advisory-tracking+][qa?]
Alias: CVE-2012-1963
Attachment #640133 - Flags: feedback?(tanvi)
Group: core-security
Flags: sec-bounty+
See Also: → CVE-2014-1591
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: