Bug 104480 - CSP: XHR from an isolated world should bypass a page's policy.
: CSP: XHR from an isolated world should bypass a page's policy.
Status: RESOLVED FIXED
: WebKit
WebCore Misc.
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
:
:
:
: 85558
  Show dependency treegraph
 
Reported: 2012-12-09 04:33 PST by
Modified: 2013-01-04 11:10 PST (History)


Attachments
Patch (13.32 KB, patch)
2012-12-10 03:03 PST, Mike West
no flags Review Patch | Details | Formatted Diff | Diff
Patch (17.96 KB, patch)
2013-01-04 00:19 PST, Mike West
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2012-12-09 04:33:04 PST
1. Yay! Facebook is using CSP!
2. Oh noes! Facebook extensions are breaking!

See: http://stackoverflow.com/questions/13786918/refused-to-connect-to-url-because-it-violates-the-following-content-security-p for some detail.

In a nutshell, we should do the same check for `connect-src` that we're currently doing for CachedResourceLoader.
------- Comment #1 From 2012-12-10 03:03:17 PST -------
Created an attachment (id=178501) [details]
Patch
------- Comment #2 From 2012-12-10 03:06:25 PST -------
Hey Eric! Are you a good person to review this patch in Adam's stead?
------- Comment #3 From 2012-12-10 11:45:11 PST -------
Sadly, no.  This really should wait for Adam's return.  Just add a note to your calendar to email him Jan 3rd. :)
------- Comment #4 From 2012-12-10 11:46:09 PST -------
If we absolutely need a fix before then, I can review it, but my understanding of the CSP security model is not nearly as deep as Adam's.
------- Comment #5 From 2012-12-10 12:28:22 PST -------
(In reply to comment #4)
> If we absolutely need a fix before then, I can review it, but my understanding of the CSP security model is not nearly as deep as Adam's.

Hrm. We don't absolutely need anything. :) This just jumped onto my radar now that Facebook launched CSP support and we're getting more bug reports.

This patch doesn't do anything really new, it simply applies the check we added to CachedResourceLoader to the areas covered by `connect-src` (WebSockers, XHR, and EventSource). I was hoping it'd be an easy patch to review, but I agree with you completely that we don't want to make mistakes in this area.

*shrug* I'll get in line for Adam's return. He really is the right guy to review.
------- Comment #6 From 2012-12-11 13:01:45 PST -------
(From update of attachment 178501 [details])
Attachment 178501 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15257797

New failing tests:
http/tests/security/isolatedWorld/bypass-main-world-csp-for-xhr.html
------- Comment #7 From 2012-12-13 03:21:27 PST -------
Thanks guys for handling this, this is really a major issue for many extensions
------- Comment #8 From 2012-12-13 05:35:08 PST -------
Thanks also! I am the topic starter of the topic Mike West referenced in his initial post, and am grateful he went and created this bug report a few minutes later. :) Thanks guys!
------- Comment #9 From 2013-01-04 00:19:30 PST -------
Created an attachment (id=181279) [details]
Patch
------- Comment #10 From 2013-01-04 09:53:57 PST -------
(From update of attachment 181279 [details])
Great!
------- Comment #11 From 2013-01-04 11:10:31 PST -------
(From update of attachment 181279 [details])
Clearing flags on attachment: 181279

Committed r138817: <http://trac.webkit.org/changeset/138817>
------- Comment #12 From 2013-01-04 11:10:37 PST -------
All reviewed patches have been landed.  Closing bug.