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 573317 UX and Extensions API confusion when file: URLs have hostnames
Starred by 4 users Reported by antonio....@gmail.com, Dec 30 2015 Back to list
Status: Fixed
Owner:
Closed: Feb 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security



Sign in to add a comment
UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/47.0.2526.106 Safari/537.36

Steps to reproduce the problem:
1. Save the attached html (chrome.html) in your hard disk 
For the record the html looks like

<h1>Hi</h1>
<script> alert(document.domain)</script>

2. now supposing the file is saved under (in MacOS) /Users/xxx/Downloads/chrome.html open the file from hard disk in this way 

file://mail.google.com/Users/xxx/Downloads/chrome.html

mail.google.com is arbitrary . This can be any domain (hence is universal)

3. Observe the document.domain alerted is mail.google.com !!! (see alert.jpg)

4. observe the cookies transported are the one associated with *.google.com domain !! (cookie.jpg)

What is the expected behavior?
document.domain is null

What went wrong?
document.domain is set by the attacker !!

Did this work before? N/A 

Chrome version: 47.0.2526.106  Channel: stable
OS Version: OS X 10.9.5
Flash Version: Shockwave Flash 20.0 r0
 
alert.png
83.6 KB View Download
chrome.html
54 bytes View Download
cookie.png
231 KB View Download
Labels: -Pri-2 -OS-Mac Pri-1 OS-All Security_Severity-Medium Security_Impact-Stable
Status: Available
Thanks for the report. I've been able to reproduce this.
Project Member Comment 2 by clusterf...@chromium.org, Jan 4 2016
Labels: Missing_Owner-1
Cc: palmer@chromium.org
Labels: -Missing_Owner-1 Cr-Blink-DOM M-49
Owner: palmer@chromium.org
Status: Assigned
palmer -- Can you take a look at this, or find an appropriate owner?  I suspect we should be dropping the domain from file:// URLs.
Cc: jochen@chromium.org meacer@chromium.org
Labels: Cr-Platform-Extensions-API Cr-Blink-JavaScript
Hmm, there is something odd going on. I have created an attack page like so:
<iframe src="https://example.com/" id="goat"></iframe>

<script>
var goat = document.getElementById("goat")
console.log("domain: " + document.domain)
console.log("cookie: " + document.cookie)
console.log("iframe document: " + goat.contentWindow.document)
</script>

(load this as file://example.com/home/you/Downloads/test.html)

Although document.domain is that of another origin, I get the following output in the console: (see screenshot)

If you swap "example.com" in the attack page and in the URL to a domain with cookies (e.g. accounts.google.com), you do get to see cookies in the Origin Info Bubble, but document.cookie is still empty. (See 2nd screenshot)

So, Chrome's UI, and Chrome's Extensions API, are showing/providing cookies for the named domain. But JavaScript can't get at them, and script in the attack page cannot actually control the other origin. So I'm not quite sure how much of an open-web vulnerability this really is? (Even though it's definitely wrong behavior, and exposes too much to extensions!)

Hostnames are a legitimate part of file: URLs (weird, but true); instead of removing them, I think we should make sure JavaScript gets null for document.domain in a file: origin, and that the UX and Extensions API also see null (and hence act accordingly). However, I am not sure if I am the best person to do that.

+jochen for JavaScript/DOM bindings
+meacer for Extension APIs
file-problem-2.png
55.8 KB View Download
Sigh, forgot the 1st screenshot.
file-problem.png
34.7 KB View Download
Cc: mkwst@chromium.org
Comment 7 by mkwst@chromium.org, Jan 7 2016
As Chris notes, this doesn't sound like a UXSS. The `file:` URL has a unique origin, and doesn't gain access to things that it frames, nor does it gain access to cookies on the hostname it asserts, nor are cookies transmitted over the wire as no network connection is actually made. As Chris also noted, hostnames are indeed part of `file:` URLs. I wouldn't mind removing that (unused?) part of the platform, honestly, but it's not clear that we're vulnerable because of it.

antonio.sanso@: Is there any JavaScript-side vulnerability you've discovered that we're missing here? If not, then I don't think this is actually a bug.
Re: #7: There are definitely bugs here, even if not *vulnerabilities*. I really don't think the UX or Extensions API should be showing/providing cookies for the named host. So we at least would need to fix that.
This is almost certainly due to the fact that stuff outside of WebKit uses GURL::GetOrigin() to get the security origin. It looks like the implementation just clears the path and several other fields, and returns the result: for a file:// URL, this will (incorrectly) leave the `domain'.
Wow, it looks like a lot of stuff needs to change from url->GetOrigin to Origin(url).

/ssd/chromium/src $ search -n '\.cc$' -C 'GetOrigin\(' extensions/ -v                                                                                             
extensions/common/url_pattern_set.cc:155:  DCHECK_EQ(origin.GetOrigin(), origin);
extensions/common/url_pattern_set.cc:158:  if (origin_pattern.Parse(origin.GetOrigin().spec()) !=
extensions/common/url_pattern_set.cc
extensions/browser/guest_view/extension_options/extension_options_guest.cc:234:    if (params.url.GetOrigin() != options_page_.GetOrigin()) {
extensions/browser/guest_view/extension_options/extension_options_guest.cc
extensions/browser/guest_view/extension_view/extension_view_guest.cc:49:      (url.GetOrigin() != extension_url_.GetOrigin());
extensions/browser/guest_view/extension_view/extension_view_guest.cc:138:  if (attached() && (params.url.GetOrigin() != url_.GetOrigin())) {
extensions/browser/guest_view/extension_view/extension_view_guest.cc
extensions/browser/api/web_request/web_request_permissions.cc:133:             url.GetOrigin() == extension->url()))) {
extensions/browser/api/web_request/web_request_permissions.cc
extensions/components/javascript_dialog_extensions_client/javascript_dialog_extension_client_impl.cc:66:        web_contents->GetLastCommittedURL().GetOrigin() == origin_url) {
extensions/components/javascript_dialog_extensions_client/javascript_dialog_extension_client_impl.cc
extensions/renderer/programmatic_script_injector.cc:135:            effective_url_.GetOrigin().spec());
extensions/renderer/programmatic_script_injector.cc
extensions/renderer/file_system_natives.cc:45:  std::string name(storage::GetIsolatedFileSystemName(context_url.GetOrigin(),
extensions/renderer/file_system_natives.cc:57:      context_url.GetOrigin(), file_system_id, optional_root_name));
extensions/renderer/file_system_natives.cc

Many more if you search in chrome/browser/ui.

Who wants to help refactor these? :)
Cc: n...@chromium.org nasko@chromium.org alex...@chromium.org
+some other site isolation folks

I don't think we make any security decisions based on GURL::GetOrigin(), even for OOPI, but I'm pretty sure we want this to work and not be subtly incorrect.

(Also, given the Blink-Chrome repo merge, maybe we should consider merging KURL and GURL? And maybe reconsider merging SecurityOrigin stuff too...)
Cc: markusheintz@chromium.org
Some CLs in progress:

https://codereview.chromium.org/1569963002
https://codereview.chromium.org/1567173002/
Labels: -Security_Severity-Medium Security_Severity-Low Cr-Security-UX
Summary: UX and Extensions API confusion when file: URLs have hostnames (was: Universal SOP bypass)
Updating to Severity: Low since, AFAICT, the worst problem would be if the person had a malicious extension installed and the cookies leaked to that extension when the browser navigated to a file: URL with a hostname matching that of a domain with cookies. That would indeed be bad, but takes this bug out of the "available to all open-web attackers".

Leaving as Pri-1 though, since the code cleanup job looks to be important. And keep an eye open for new vulnerability scenarios as we do that cleanup (and then re-raise the severity of this bug if necessary).
Project Member Comment 14 by bugdroid1@chromium.org, Jan 11 2016
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/58cb65370efa360e924ca222af05c3bef09e168c

commit 58cb65370efa360e924ca222af05c3bef09e168c
Author: palmer <palmer@chromium.org>
Date: Mon Jan 11 19:46:47 2016

Make the Permission Bubble Manager use a correct same-origin check.

GURL::GetOrigin does not do the right thing for all types of URLs.

BUG= 573317 

Review URL: https://codereview.chromium.org/1569963002

Cr-Commit-Position: refs/heads/master@{#368641}

[modify] http://crrev.com/58cb65370efa360e924ca222af05c3bef09e168c/chrome/browser/ui/website_settings/permission_bubble_manager.cc

Project Member Comment 15 by bugdroid1@chromium.org, Jan 11 2016
Status: Started
Comment 17 by habl...@google.com, Jan 22 2016
Labels: -Cr-Blink-JavaScript
Cc: benwells@chromium.org rdevlin....@chromium.org
Project Member Comment 19 by bugdroid1@chromium.org, Feb 3 2016
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5c437bcc7a51edbef45242c5173cf7871fde2866

commit 5c437bcc7a51edbef45242c5173cf7871fde2866
Author: palmer <palmer@chromium.org>
Date: Wed Feb 03 23:21:36 2016

Make extensions use a correct same-origin check.

GURL::GetOrigin does not do the right thing for all types of URLs.

BUG= 573317 

Review URL: https://codereview.chromium.org/1658913002

Cr-Commit-Position: refs/heads/master@{#373381}

[modify] http://crrev.com/5c437bcc7a51edbef45242c5173cf7871fde2866/extensions/browser/api/web_request/web_request_permissions.cc
[modify] http://crrev.com/5c437bcc7a51edbef45242c5173cf7871fde2866/extensions/browser/guest_view/extension_options/extension_options_guest.cc
[modify] http://crrev.com/5c437bcc7a51edbef45242c5173cf7871fde2866/extensions/browser/guest_view/extension_view/extension_view_guest.cc
[modify] http://crrev.com/5c437bcc7a51edbef45242c5173cf7871fde2866/extensions/common/url_pattern_set.cc
[modify] http://crrev.com/5c437bcc7a51edbef45242c5173cf7871fde2866/extensions/components/javascript_dialog_extensions_client/javascript_dialog_extension_client_impl.cc
[modify] http://crrev.com/5c437bcc7a51edbef45242c5173cf7871fde2866/extensions/renderer/file_system_natives.cc
[modify] http://crrev.com/5c437bcc7a51edbef45242c5173cf7871fde2866/extensions/renderer/programmatic_script_injector.cc
[modify] http://crrev.com/5c437bcc7a51edbef45242c5173cf7871fde2866/url/origin.cc
[modify] http://crrev.com/5c437bcc7a51edbef45242c5173cf7871fde2866/url/origin.h

Status: Fixed
AFAICT, this bug is fixed for the Cookie and Extension cases. Please re-open if not.
Project Member Comment 21 by clusterf...@chromium.org, Feb 4 2016
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: -M-49 reward-topanel M-50 Release-0-M50
Low severity bugs roll in off trunk - this will land on stable channel with M50.
Project Member Comment 23 by bugdroid1@chromium.org, Apr 12 2016
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8e4a0b36ac3989db1afb69dff442fdc85af047da

commit 8e4a0b36ac3989db1afb69dff442fdc85af047da
Author: battre <battre@chromium.org>
Date: Tue Apr 12 15:49:42 2016

Unhide "Zombie-cookies".

Some cookies would not show up in chrome://settings/cookies or
the site info dialog because the source_ in the CanonicalCookie
is not persisted beyond browser restarts.
https://crrev.com/a6acbbcaccdb4085f17d0eef1f266d48fa4baff0 created
a filter to allow only access to HTTP or HTTPS cookies. This CL
moves the filtering logic a few lines behind a fall back path.

R=msramek@chromium.org,palmer@chromium.org,markusheintz@chromium.org
BUG= 573317 , 601582 

Review URL: https://codereview.chromium.org/1870973002

Cr-Commit-Position: refs/heads/master@{#386695}

[modify] https://crrev.com/8e4a0b36ac3989db1afb69dff442fdc85af047da/chrome/browser/browsing_data/cookies_tree_model.cc
[modify] https://crrev.com/8e4a0b36ac3989db1afb69dff442fdc85af047da/chrome/browser/browsing_data/cookies_tree_model_unittest.cc

Labels: CVE-2016-1658
Thanks for the report. How would you like to be credited when we mention this in Chrome's release notes?
hi there,

thanks a lot to you.
If possible it would be nice to credit as:

Antonio Sanso (@asanso) of Adobe
Labels: -reward-topanel reward-unpaid Reward-500
Thanks again for the report! This one qualified for a $500 reward.
Project Member Comment 28 by bugdroid1@chromium.org, Apr 15 2016
Labels: merge-merged-2704
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/44959317e61b85de3c25fdb066cbd8481871f637

commit 44959317e61b85de3c25fdb066cbd8481871f637
Author: Dominic Battre <battre@chromium.org>
Date: Fri Apr 15 07:32:20 2016

Unhide "Zombie-cookies".

Some cookies would not show up in chrome://settings/cookies or
the site info dialog because the source_ in the CanonicalCookie
is not persisted beyond browser restarts.
https://crrev.com/a6acbbcaccdb4085f17d0eef1f266d48fa4baff0 created
a filter to allow only access to HTTP or HTTPS cookies. This CL
moves the filtering logic a few lines behind a fall back path.

R=msramek@chromium.org,palmer@chromium.org,markusheintz@chromium.org
BUG= 573317 , 601582 

Review URL: https://codereview.chromium.org/1870973002

Cr-Commit-Position: refs/heads/master@{#386695}
(cherry picked from commit 8e4a0b36ac3989db1afb69dff442fdc85af047da)

Review URL: https://codereview.chromium.org/1891153002 .

Cr-Commit-Position: refs/branch-heads/2704@{#69}
Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251}

[modify] https://crrev.com/44959317e61b85de3c25fdb066cbd8481871f637/chrome/browser/browsing_data/cookies_tree_model.cc
[modify] https://crrev.com/44959317e61b85de3c25fdb066cbd8481871f637/chrome/browser/browsing_data/cookies_tree_model_unittest.cc

Labels: -reward-unpaid reward-inprocess
Our finance team should reach out within 7 days to collect payment details. If that doesn't happen, please email me at timwillis@ or update this bug.

Thanks again for your report!
Project Member Comment 30 by bugdroid1@chromium.org, May 6 2016
Labels: merge-merged-2661
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b7c0ea69b403ef18e1381c3c78b5a9aea336aa70

commit b7c0ea69b403ef18e1381c3c78b5a9aea336aa70
Author: Dominic Battre <battre@chromium.org>
Date: Fri May 06 19:26:55 2016

Unhide "Zombie-cookies".

Some cookies would not show up in chrome://settings/cookies or
the site info dialog because the source_ in the CanonicalCookie
is not persisted beyond browser restarts.
https://crrev.com/a6acbbcaccdb4085f17d0eef1f266d48fa4baff0 created
a filter to allow only access to HTTP or HTTPS cookies. This CL
moves the filtering logic a few lines behind a fall back path.

R=msramek@chromium.org,palmer@chromium.org,markusheintz@chromium.org
BUG= 573317 , 601582 

Review URL: https://codereview.chromium.org/1870973002

Cr-Commit-Position: refs/heads/master@{#386695}
(cherry picked from commit 8e4a0b36ac3989db1afb69dff442fdc85af047da)

Review URL: https://codereview.chromium.org/1947933006 .

Cr-Commit-Position: refs/branch-heads/2661@{#664}
Cr-Branched-From: ef6f6ae5e4c96622286b563658d5cd62a6cf1197-refs/heads/master@{#378081}

[modify] https://crrev.com/b7c0ea69b403ef18e1381c3c78b5a9aea336aa70/chrome/browser/browsing_data/cookies_tree_model.cc
[modify] https://crrev.com/b7c0ea69b403ef18e1381c3c78b5a9aea336aa70/chrome/browser/browsing_data/cookies_tree_model_unittest.cc

Project Member Comment 31 by sheriffbot@chromium.org, May 12 2016
Labels: -Restrict-View-SecurityNotify
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
Project Member Comment 32 by sheriffbot@chromium.org, Oct 1 2016
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
Project Member Comment 33 by sheriffbot@chromium.org, Oct 2 2016
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
Labels: allpublic
Components: -Security>UX
Labels: Team-Security-UX
Security>UX component is deprecated in favor of the Team-Security-UX label
Sign in to add a comment