New issue
Advanced search Search tips

Issue 810220 link

Starred by 3 users

Issue metadata

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



Sign in to add a comment

Security: Extension with <all_urls> permission can read arbitrary local files and chrome:// pages

Reported by francois...@gmail.com, Feb 8 2018

Issue description

VULNERABILITY DETAILS
An extension with the <all_urls> permission can read any file on the file system and send its content to a remote host, using chrome.tabs.captureVisibleTab.

VERSION
Chrome Version: 64.0.3282.140 + stable
Operating System: Windows 10 Home, Version 1709

REPRODUCTION CASE
The attachment is a POC extension that upon loading will :
1- Create a new tab (using tabs.create) with its URL set to the value of the fileToFetch constant (DEFAULT set to file:///c:/) ;
2- Listen for tabs updates and try to take a screenshot (using chrome.tabs.captureVisibleTab) of the newly opened tab
3- If the screenshot fails (which will occur upon opening of file:///c:/ using tabs.create), reload the tab. This will cause the tab to be updated and a new screenshot to be taken ;
4- When the screenshot is made the image data is displayed in extension console and send it to the exfiltration endpoint specified by the value of exfiltrationEndpoint constant.
5- The opened tab is closed

From there it is easy to create a command and control server which will enable an attacker to navigate throught the file system and fetch arbitrary files.
 
SpyTab.zip
1.1 KB Download
Components: Platform>Extensions>API
Labels: OS-Chrome OS-Fuchsia OS-Linux OS-Mac OS-Windows
Labels: ReleaseBlock-Stable M-65 Security_Severity-Medium Security_Impact-Stable
Owner: wjmaclean@chromium.org
Status: Assigned (was: Unconfirmed)
wjmaclean@, can you please take a look (could this be related to https://chromium.googlesource.com/chromium/src/+/f89035216b627283b79731c3e6a7957707ed9034)
Project Member

Comment 3 by sheriffbot@chromium.org, Feb 8 2018

Labels: Pri-1
It's not clear to me what part of this isn't working as expected? 

An extension with <all_URLs> permission can navigate to any URL and can read content directly from any page loaded from any URL. This is an inherently powerful permission.

The use of reload() is a bit of a red-herring here. The initial attempt to screenshot the tab fails (likely because the surface isn't ready) but this can be worked around with setTimeout, e.g.

----
  extObj.tabs.onUpdated.addListener((tabid,c,tab) => {
    if (c.status === "complete")
      setTimeout(function(){extObj.tabs.captureVisibleTab(tab.windowId,captureCallback(tab.id)) }, 200);
  });
----
In my opinion the thing that is not working properly is the fact that an extension can create a new tab to any resources. 

For example, Firefox restrict URL of resource that can be opened with tabs.create (see https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/tabs/create).

Moreover, it should be considered to further restrict screenshot capability if the tabs permission is not granted. No requiring it to take screenshot is "like" enabling injecting script to fetch page content.

Finally, screenshot should be disabled no matter the granted permission for chrome url and file on the file system. If not enforced, a malicious extension could only wait for you to access chrome://history/ to retrieve the browsing history without  requesting the history or the topsite permission. Therefore, leading to an extension "privilege" escalation.
Cc: devlin@chromium.org
Summary: Security: Extension with <all_urls> permission can read arbitrary local files and chrome:// pages (was: Security: Extension with <all_urls> permission can read arbitrary file of the file system)
The original POC continues to work when "Allow access to file URLs" is disabled within the extension's settings. /That/ seems like something that could be addressed by restricting captureVisibleTab() when the tab is on a file URI. Perhaps tab.create("file:///") and its ilk should be forbidden in that circumstance as well.

> the thing that is not working properly is the fact that an extension can 
> create a new tab to any resources 

It's correct to note that the tabs.create() method (https://developer.chrome.com/extensions/tabs#method-create) is powerful and it could be made less powerful by restricting it from navigating to chrome: or file: URLs. As you note, however, this doesn't really prevent spying by malicious extensions which can simply wait for the user to perform their own navigations.

> it should be considered to further restrict screenshot 
> capability if the tabs permission is not granted.

The documentation for https://developer.chrome.com/extensions/tabs#method-captureVisibleTab explicitly notes that it requires the powerful <all_urls> permission, and the documentation for the tabs API notes "The majority of the chrome.tabs API can be used without declaring any permission. However, the "tabs" permission is required in order to populate the url, title, and favIconUrl properties of Tab." I expect the idea is that "<all_urls>" match pattern supersedes the less powerful "tabs" permission, and thus the latter need not be explicitly declared. 
Yeah, we should really be checking that the extension has permission to access the given page.  That will solve the case of capturing chrome:// and file:// (without permission) urls.  It seems like we should just be able to put this check in the TabsCaptureVisibleTabs function.
The TabsCaptureVisibleTabFunction::GetWebContentsForID function calls PermissionsData::CanCaptureVisiblePage. That function checks for the <all_urls> permission but, interestingly, even if all_urls permission isn't present, it then checks

 GetTabSpecificPermissions(tab_id)->HasAPIPermission(APIPermission::kTab)

meaning an extension with "activeTab" permission may use the API despite the claims in the documentation. (The captured image also includes all of the tab's subframes, even if they're cross-origin.) 

Comment 9 by gov...@chromium.org, Feb 13 2018

M65 Stable promotion is coming VERY soon. Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and request a merge  into the release branch ASAP. Thank you.
Owner: elawrence@chromium.org
Re-assigning, since it looks like elawrence@ is looking into this.
Cc: elawrence@chromium.org
Owner: ----
I'm not really an appropriate owner for this, I think we need someone on the extensions team to own both the overall policy/plan and the specific fixes to TabsCaptureVisibleTabFunction. 

@devlin, do you have suggestions for who that might be?
Owner: rdevlin....@chromium.org
Assigning to rdevlin.cronin@ for now.
Labels: -ReleaseBlock-Stable
Removing ReleaseBlock-Stable, but once we have a fix in hand we should consider if it's safe enough to merge to 65 to be picked up in any stable refreshes.
Project Member

Comment 14 by sheriffbot@chromium.org, Feb 24 2018

rdevlin.cronin: Uh oh! This issue still open and hasn't been updated in the last 15 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers?

If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one?

If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 15 by sheriffbot@chromium.org, Mar 10 2018

rdevlin.cronin: Uh oh! This issue still open and hasn't been updated in the last 29 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers?

If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one?

If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
I'm diving into this now; sorry for the delay.

elawrence@: is this security_impact-Medium?  Capturing (but not executing script on) these pages when the extension has either been explicitly invoked or has all urls permission seems like Low to me, but I'll gladly defer to your expertise.
Cc: -devlin@chromium.org mea...@chromium.org
+meacer

meacer@ and elawrence@, still curious for your take on security_impact level here.

Also, another note: I think this might be actively used by some screenshot extensions to take screenshots of e.g. settings pages (useful for filing bugs).  Are we okay breaking that use case?
meacer is more of an expert on our Extensions system than I am, but I do think this currently rates Severity_Medium, based on the current Severity guidelines[1]. If the attack were limited to pages which the user explicitly interacted (e.g. clicked a BrowserAction button), I'd be comfortable with Severity_Low, but I think the circumvention of the "Allow access to file URLs" checkbox is a pretty significant violation of user's expectations.

[1] https://chromium.googlesource.com/chromium/src/+/lkcr/docs/security/severity-guidelines.md#Medium-severity

In terms of the compatibility question, can we take the same approach-- e.g. an explicit user invocation via a BrowserAction allows the screenshot, but the all_urls permission alone does not?
Cc: karandeepb@chromium.org
Discussed with meacer@ offline, and he suggested similar behavior.  So, what we have is:

<all_urls>: captureVisibleTab disallowed on chrome:// and file://
<all_urls> + allowed file access through settings: captureVisibleTab disallowed on chrome://, allowed on file://
activeTab granted: captureVisibleTab allowed on chrome://, ?? on file://
activeTab granted + allowed file access through settings: captureVisibleTab allowed on chrome:// and file://

?? -> This is dependent on  issue 816685 , which will require file access in settings before granting active tab.
Project Member

Comment 20 by bugdroid1@chromium.org, Apr 6

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

commit 0aca6bc05a263ea9eafee515fc6ba14da94c1964
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Fri Apr 06 19:19:23 2018

[Extensions] Restrict tabs.captureVisibleTab()

Modify the permissions for tabs.captureVisibleTab(). Instead of just
checking for <all_urls> and assuming its safe, do the following:
- If the page is a "normal" web page (e.g., http/https), allow the
  capture if the extension has activeTab granted or <all_urls>.
- If the page is a file page (file:///), allow the capture if the
  extension has file access *and* either of the <all_urls> or
  activeTab permissions.
- If the page is a chrome:// page, allow the capture only if the
  extension has activeTab granted.

Bug:  810220 

Change-Id: I1e2f71281e2f331d641ba0e435df10d66d721304
Reviewed-on: https://chromium-review.googlesource.com/981195
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548891}
[modify] https://crrev.com/0aca6bc05a263ea9eafee515fc6ba14da94c1964/chrome/browser/extensions/active_tab_unittest.cc
[modify] https://crrev.com/0aca6bc05a263ea9eafee515fc6ba14da94c1964/chrome/browser/extensions/api/tabs/tabs_api.cc
[modify] https://crrev.com/0aca6bc05a263ea9eafee515fc6ba14da94c1964/chrome/common/extensions/permissions/permissions_data_unittest.cc
[modify] https://crrev.com/0aca6bc05a263ea9eafee515fc6ba14da94c1964/chrome/test/data/extensions/api_test/tabs/capture_visible_tab/test_disabled.js
[modify] https://crrev.com/0aca6bc05a263ea9eafee515fc6ba14da94c1964/chrome/test/data/extensions/api_test/tabs/capture_visible_tab_null_window/background.js
[modify] https://crrev.com/0aca6bc05a263ea9eafee515fc6ba14da94c1964/extensions/common/permissions/permissions_data.cc
[modify] https://crrev.com/0aca6bc05a263ea9eafee515fc6ba14da94c1964/extensions/common/permissions/permissions_data.h

Project Member

Comment 21 by sheriffbot@chromium.org, Apr 18

Labels: -M-65 M-66
Status: Fixed (was: Assigned)
I think this should be fixed with #20 (which is already in 67).  I don't think we'll merge this back to 66, but I'll leave that to release managers and security experts to decide.
Project Member

Comment 23 by sheriffbot@chromium.org, Apr 19

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.
*********************************
Cc: awhalley@chromium.org
Congrats francois.lajeunesse.robert@! The Chrome VRP panel has decided to award $2,000 for this report.  A member of our finance team will be in touch to arrange payment. Also, how would you like to be credited in the Chrome release notes?
For release notes credit, mentionning my name Fran├žois Lajeunesse-Robert is sufficient.

Thank you
Project Member

Comment 28 by sheriffbot@chromium.org, Apr 27

Labels: Merge-Request-67
Project Member

Comment 29 by sheriffbot@chromium.org, Apr 27

Labels: -Merge-Request-67 Merge-Review-67 Hotlist-Merge-Review
This bug requires manual review: M67 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), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -reward-unpaid reward-inprocess
+awhalley@ for M67 merge review. 
Commit from #20 initially landed in 67.0.3392.0
Labels: -Merge-Review-67
Removing "Merge-Review-67" label per comment #32.
Labels: Release-0-M67
Labels: CVE-2018-6138 CVE_description-missing
Project Member

Comment 36 by sheriffbot@chromium.org, Jul 26

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