New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac , Fuchsia
Pri: 1
Type: Bug-Security



Sign in to add a comment

Cross-origin Shared Worker

Reported by s.h.h.n....@gmail.com, Nov 20 2017

Issue description

UserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/62.0.3202.94 Safari/537.36

Steps to reproduce the problem:
1. Go to https://test.shhnjk.com/2.html
2. Open another tab and go to https://vuln.shhnjk.com/2.html
3. Click on stop button on one tab

What is the expected behavior?
other tab keeps counting and does not stop (SharedWorker won't be established between cross-origin)

What went wrong?
In https://html.spec.whatwg.org/multipage/workers.html#sharedworker, step 11.2's note of "When the SharedWorker(scriptURL, options) constructor is invoked:"  says

data: URLs create a worker with an opaque origin. Both the constructor origin and constructor url are compared so the same data: URL can be used within an origin to get to the same SharedWorkerGlobalScope object, but cannot be used to bypass the same origin restriction.

This is not the case in Chrome and cross-origin site can create SharedWorker using data URL SharedWorker script. 

Did this work before? N/A 

Chrome version: 62.0.3202.94  Channel: stable
OS Version: 10.0
Flash Version:
 

Comment 1 by mmoroz@chromium.org, Nov 20 2017

Components: Blink>ServiceWorker
Labels: Security_Impact-Stable OS-Linux
Owner: nhiroki@chromium.org
Status: Assigned (was: Unconfirmed)
Hiroki, could you please take a look?

It reproduces on Linux as well.
Components: -Blink>ServiceWorker Blink>Workers
Status: Started (was: Assigned)
Thank you for reporting this.

Looks like URL matching logic (SharedWorkerInstance::Matches()) is not correct for Data URL. Data URL has an empty origin and can pass the logic. Probably we should compare "constructor origin" set to "outside settings's origin"[1] instead of a given URL's origin.

[1] https://html.spec.whatwg.org/multipage/workers.html#run-a-worker

Cc: falken@chromium.org kinuko@chromium.org
Labels: -Pri-2 Pri-1
Bumping up the priority because this is a security issue to bypass the same origin policy...

Comment 5 by falken@chromium.org, Nov 21 2017

Cc: andypaicu@chromium.org
Related to  issue 270979  and https://www.chromestatus.com/feature/5633342665916416?

Seems like https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/6otx9aZlwEo/ddfnO5gTCAAJ had a lot of discussion about the security model.
falken@: Thank you for sharing information!

WIP CL (I'll add tests):
https://chromium-review.googlesource.com/c/chromium/src/+/781539

Comment 7 by mmoroz@chromium.org, Nov 21 2017

Labels: Security_Severity-High M-63

Comment 8 by palmer@chromium.org, Nov 28 2017

Labels: OS-Android OS-Chrome OS-Fuchsia OS-Mac
nhiroki: Friendly ping. :) 
I'm still working on the CL to fix this in M64:
https://chromium-review.googlesource.com/c/chromium/src/+/781539
Project Member

Comment 10 by bugdroid1@chromium.org, Nov 30 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/018bb6d300c11acb953d51ef3cbec4cdcaf4a652

commit 018bb6d300c11acb953d51ef3cbec4cdcaf4a652
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Thu Nov 30 03:31:37 2017

SharedWorker: Introduce "constructor origin" concept defined in the spec

This CL introduces the "constructor origin" concept defined in the HTML spec:
https://html.spec.whatwg.org/multipage/workers.html#concept-sharedworkerglobalscope-constructor-origin

Bug:  787103 
Change-Id: I273629760bb34e0c24f1c4d023e66e146a476407
Reviewed-on: https://chromium-review.googlesource.com/781539
Reviewed-by: Raymes Khoury <raymes@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520417}
[modify] https://crrev.com/018bb6d300c11acb953d51ef3cbec4cdcaf4a652/chrome/browser/browsing_data/browsing_data_shared_worker_helper.cc
[modify] https://crrev.com/018bb6d300c11acb953d51ef3cbec4cdcaf4a652/chrome/browser/browsing_data/browsing_data_shared_worker_helper.h
[modify] https://crrev.com/018bb6d300c11acb953d51ef3cbec4cdcaf4a652/chrome/browser/browsing_data/browsing_data_shared_worker_helper_unittest.cc
[modify] https://crrev.com/018bb6d300c11acb953d51ef3cbec4cdcaf4a652/chrome/browser/browsing_data/cookies_tree_model.cc
[modify] https://crrev.com/018bb6d300c11acb953d51ef3cbec4cdcaf4a652/chrome/browser/browsing_data/mock_browsing_data_shared_worker_helper.cc
[modify] https://crrev.com/018bb6d300c11acb953d51ef3cbec4cdcaf4a652/chrome/browser/browsing_data/mock_browsing_data_shared_worker_helper.h
[modify] https://crrev.com/018bb6d300c11acb953d51ef3cbec4cdcaf4a652/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/018bb6d300c11acb953d51ef3cbec4cdcaf4a652/chrome/browser/chrome_content_browser_client.h
[modify] https://crrev.com/018bb6d300c11acb953d51ef3cbec4cdcaf4a652/chrome/browser/content_settings/tab_specific_content_settings.cc
[modify] https://crrev.com/018bb6d300c11acb953d51ef3cbec4cdcaf4a652/chrome/browser/content_settings/tab_specific_content_settings.h
[modify] https://crrev.com/018bb6d300c11acb953d51ef3cbec4cdcaf4a652/content/browser/devtools/shared_worker_devtools_manager_unittest.cc
[modify] https://crrev.com/018bb6d300c11acb953d51ef3cbec4cdcaf4a652/content/browser/shared_worker/shared_worker_instance.cc
[modify] https://crrev.com/018bb6d300c11acb953d51ef3cbec4cdcaf4a652/content/browser/shared_worker/shared_worker_instance.h
[modify] https://crrev.com/018bb6d300c11acb953d51ef3cbec4cdcaf4a652/content/browser/shared_worker/shared_worker_instance_unittest.cc
[modify] https://crrev.com/018bb6d300c11acb953d51ef3cbec4cdcaf4a652/content/browser/shared_worker/shared_worker_service_impl.cc
[modify] https://crrev.com/018bb6d300c11acb953d51ef3cbec4cdcaf4a652/content/browser/shared_worker/shared_worker_service_impl.h
[modify] https://crrev.com/018bb6d300c11acb953d51ef3cbec4cdcaf4a652/content/browser/shared_worker/shared_worker_service_impl_unittest.cc
[modify] https://crrev.com/018bb6d300c11acb953d51ef3cbec4cdcaf4a652/content/public/browser/content_browser_client.cc
[modify] https://crrev.com/018bb6d300c11acb953d51ef3cbec4cdcaf4a652/content/public/browser/content_browser_client.h
[modify] https://crrev.com/018bb6d300c11acb953d51ef3cbec4cdcaf4a652/content/public/browser/shared_worker_service.h
[add] https://crrev.com/018bb6d300c11acb953d51ef3cbec4cdcaf4a652/third_party/WebKit/LayoutTests/external/wpt/workers/data-url-shared-window.html
[modify] https://crrev.com/018bb6d300c11acb953d51ef3cbec4cdcaf4a652/third_party/WebKit/LayoutTests/external/wpt/workers/data-url-shared.html

Labels: -M-63 M-64
Status: Fixed (was: Started)
Labels: -OS-Android
Android doesn't support SharedWorker.
Project Member

Comment 13 by sheriffbot@chromium.org, Nov 30 2017

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: reward-topanel
Labels: -reward-topanel reward-unpaid reward-2000
*** Boilerplate reminders! ***
Please do NOT publicly disclose details until a fix has been released to all our users. Early public disclosure may cancel the provisional reward. Also, please be considerate about disclosure when the bug affects a core library that may be used by other products. Please do NOT share this information with third parties who are not directly involved in fixing the bug. Doing so may cancel the provisional reward. Please be honest if you have already disclosed anything publicly or to third parties. Lastly, we understand that some of you are not interested in money. We offer the option to donate your reward to an eligible charity. If you prefer this option, let us know and we will also match your donation - subject to our discretion. Any rewards that are unclaimed after 12 months will be donated to a charity of our choosing.
*********************************
Nice on s.h.h.n.j.k@! The VRP panel decided to award $2,000 for this report. Cheers!
Labels: -reward-unpaid reward-inprocess
Project Member

Comment 18 by sheriffbot@chromium.org, Dec 15 2017

Labels: Merge-Request-64
Project Member

Comment 19 by sheriffbot@chromium.org, Dec 15 2017

Labels: -Merge-Request-64 Hotlist-Merge-Review Merge-Review-64
This bug requires manual review: M64 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
abdulsyed@ - good for M64
nhiroki@ - this seems like a very large change overall and is touching some key directories. Can you please comment on whether this is absolutely needed for M64 vs waiting until M65? How well tested is this? Are we confident it won't break anything?
Labels: -Merge-Review-64
On a second look, seems like this is already in M64 and made it before branch. Removing Merge-Request label. 

https://chromium.googlesource.com/chromium/src.git/+/018bb6d300c11acb953d51ef3cbec4cdcaf4a652
Sorry for the late comment. I didn't notice the automatic merge request. Yes, this should already be in M64.

$ git find-releases 018bb6d300c11acb953d51ef3cbec4cdcaf4a652
commit 018bb6d300c11acb953d51ef3cbec4cdcaf4a652 was:
  initially in 64.0.3282.0
Labels: Release-0-M64
Labels: CVE-2018-6032
Cc: pelizzi@google.com
Project Member

Comment 27 by sheriffbot@chromium.org, Mar 8

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
Project Member

Comment 28 by sheriffbot@chromium.org, Mar 27

Labels: -M-64 M-65
Labels: CVE_description-missing

Sign in to add a comment