Status Update
Comments
ch...@google.com <ch...@google.com> #2
I could reproduce the bug behind this on version 105, although the chrome.downloads API wasn't working properly, so I couldn't get a proper POC. I wouldn't be surprised if it works in even older versions.
ch...@google.com <ch...@google.com> #3
Woah, I figured out something new that could make this XSS even more dangerous. Because the XSS in its original form is on a filesystem: page, it doesn't have access to things such as the private APIs on chrome://file-manager.
But while I was messing around, I figured that the filesystem: URL can get access to the window (and thus APIs) of the actual files app interface. Normally, this would be easy to do with window.open, because the filesystem: URL and the pure chrome:// one have the same origin. But because chrome://file-manager opens as an app, I originally thought communication would be impossible.
Steps for gaining access to the real chrome page:
1. Run something like `var FILEMANAGER = window.open("invalid:url")` from the filesystem: page with the XSS. It must be an invalid URL, and thus "unrendered by the browser". about:blank will not work, for some reason .
2. Use chrome.tabs.update() from the extension to redirect the "unrendered" tab to chrome://file-manager. This opens it as an actual tab and not as an app. But it's in a semi-broken state for some reason.
3. Use chrome.tabs.reload() from the extension, which somehow takes the file-manager tab out of this broken state.
4. `FILEMANAGER.window` (a working chrome://file-manager context) can now be accessed from the filesystem: page
By gaining this access, an extension can:
1. Use chrome.fileManagerPrivate and all _eighty-five_ of its private API functions. Creating and editing files is trivial.
2. Probably have better access to Mojo and chrome.send because the requests are now coming from a "pure" Chrome URL
A modified version of my POC extension has been attached with these new changes.
ch...@google.com <ch...@google.com> #4
Comment by ortuno@
joelhockey and I chatted this morning. There are a couple of issues/bugs that allow for the exploit, but the starting point is that we incorrectly load "filesystem:chrome://file-manager" as a proper WebUI i.e. it has a WebUIController. This is because `url::Origin("filesystem:chrome://file-manager") == url::Origin("chrome://file-manager")` so we happily load the page as a WebUI.
Before WebUIConfigMap we used to compare hosts and `GURL("filesystem://chrome://file-manager).host() != GURL("chrome://file-manager").host()`. This bug was added in 115[1]. Before then, it was possible to open filesystem:chrome://file-manager URLs but it wouldn't get access to chrome.send() or Mojo. The second POC would also not work. We verified this by changing WebUIConfigMap to not create WebUIs for filesystem urls.
I think the short term fix would be to change WebUIConfigMap::GetConfig()[2] to use a GURL instead of a url::Origin and to filter out any filesystem, and possible blob, URLs. This should at least mitigate the exploit. I can take this part.
Beyond that, there are a couple of other issues:
0. Extensions can use chrome.downloads to get the full path of a downloaded file. Then they can use that path to get the user hash which they can use in other parts of the exploit.
1. Extensions can open user files using filesystem://chrome://file-manager/external. (We couldn't figure out what made chrome://file-manager special here. Maybe it's possible to open files under other WebUIs).
2. We run filesystem:chrome:// URLs as if they were in the chrome:// origin even when they point to files outside of the WebUI's resources.
3. The fact that a filesystem:chrome:// URL can open a chrome:// tab and have access to `window` from the second tab. We're lucky the WebUI infra notices the first tab has incorrect WebUI bindings and crashes it before it can do anything.
I can take the WebUIConfig fix, but would be good if someone else familiar with WebUI took on the other issues. Nasko, do you think someone from your team could take a look at those?
[1]
[2]
ch...@google.com <ch...@google.com> #5
Comment from reporter regarding #4
First of all, I can indeed confirm that the second POC doesn't fully work on Chrome 105, and that the filesystem url crashes upon trying. Although the filesystem: page by itself can still do things like fetch Chrome URLs on 105 (see comment 2)
As for your point 0, it's worth noting that there are likely a lot of ways that this could be done. An extension could read something like file:///var/log/arc.1.log which also contains the user hash.
For your point 1, I think that chrome.fileManagerPrivate.getVolumeRoot or a similar internal is probably configured to host its OPFS filesystem on chrome://file-manager.
Also, why is the browser allowed to display filesystem:chrome:// URLs, anyway? file-manager is the only origin that uses them and it never displays them with that protocol. It's not the best solution, but I suppose you could just stop the browser from loading those URLs.
si...@google.com <si...@google.com> #6
@na...@google.com would you be able to take a look at @or...@google.com's question in c#4? Thank you.
ch...@google.com <ch...@google.com> #7
Yeah, it's still a problem that we render those URLs as chrome:// URLs in 105, but at least we don't grant it Mojo, chrome.send(), and chrome.fileManagerPrivate, like in after 115.
Point 0: Makes sense! Thanks for the context.
Point 1. Ohh interesting. That would explain why we don't see anything in the backend of the WebUI that loads the files.
ma...@gmail.com <ma...@gmail.com> #8
Ah, I still remember reading
si...@google.com <si...@google.com> #9
Temporarily assigning to @na...@google.com -- could you please take a look at @or...@google.com's question in c#4? Thank you.
or...@google.com <or...@google.com> #10
ch...@google.com <ch...@google.com> #11
or...@google.com <or...@google.com> #12
ch...@google.com <ch...@google.com> #13
or...@google.com <or...@google.com> #14
ch...@google.com <ch...@google.com> #15
ap...@google.com <ap...@google.com> #16
Branch: main
commit b46c37206ab6f8df2c2ce8f23383c967360dab55
Author: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Date: Wed Jul 19 07:42:54 2023
webui: Filter out non-chrome scheme URLs in WebUIConfigMap
url::Origin::Create() drops filesystem: and blob: from URLs so filter
those out before using Create().
Bug:
Change-Id: Ia8fd0d1048deaef346accd7b4cb64b448e8dc9f1
Reviewed-on:
Commit-Queue: Giovanni Ortuno Urquidi <ortuno@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1172220}
M chrome/browser/ash/system_extensions/system_extensions_install_manager.cc
M chrome/browser/ash/system_web_apps/apps/demo_mode_app_integration_browsertest.cc
M chrome/browser/chrome_content_browser_client.cc
M content/browser/service_worker/service_worker_context_wrapper.cc
M content/browser/webui/web_ui_browsertest.cc
A content/browser/webui/webui_config_map_unittest.cc
M content/public/browser/webui_config_map.cc
M content/public/browser/webui_config_map.h
M content/public/test/scoped_web_ui_controller_factory_registration.cc
M content/public/test/scoped_web_ui_controller_factory_registration.h
M content/test/BUILD.gn
or...@google.com <or...@google.com> #17
The CL in #16 should stop the original POC. I believe joelhockey is working on a more long term fix and the questions on the original bug have been answered elsewhere. @ch...@google.com, would you mind assigning this bug to joelhockey?
or...@google.com <or...@google.com> #19
There haven't been any issues with the CL on canary or dev, so requesting a merge to M115 and M116. This partially mitigates a security issue introduced in M115. A full mitigation is still being worked on.
ch...@google.com <ch...@google.com> #20
1. Explain how/why your merge fits within the Merge Decision Guidelines?
- Googlers:
- Partners:
2. Links to the CLs you are requesting to merge.
3. Has the change landed and been verified on ToT? If so, explain what testing has been done to limit the risk of regressions.
4. Does this change need to be merged into other active release branches (M-1, M+1)?
5. Has your Eng Prod Representative reviewed this change and confirmed that it can be sufficiently tested? (if this merge would change/add to the required testing)
- See
6. Why are these changes required in this milestone after branch?
7. Is this a new/unlaunched/in-development feature or a bug fix related to a new/unlaunched/in-development feature in any way?
8. Is the scope of this change limited to unreleased devices only? If the request is specific to an unreleased device, but the cherry-pick is not going to land in a model-specific directory, please CC a member of the SIE team to LGTM that the change will not impact any released devices.
Please contact the milestone owner if you have questions.
or...@google.com <or...@google.com> #21
- Explain how/why your merge fits within the Merge Decision Guidelines?
Partial fix for a security bug introduced in M115. Full fix is still being worked on. It would be reasonable to not merge this and only merge the full fix, but this does mitigate part of the security issue.
- Links to the CLs you are requesting to merge.
- Has the change landed and been verified on ToT? If so, explain what testing has been done to limit the risk of regressions.
Yes. The CL landed last week and has been tested on Dev. There have been no new crashes related to it.
- Does this change need to be merged into other active release branches (M-1, M+1)?
M115 and M116.
- Has your Eng Prod Representative reviewed this change and confirmed that it can be sufficiently tested? (if this merge would change/add to the required testing)
- See
http://go/croscomponents to find your representative (if you do not have access, please work with your Google contact)
Doesn't require testing outside automated tests.
- Why are these changes required in this milestone after branch?
Security fix.
- Is this a new/unlaunched/in-development feature or a bug fix related to a new/unlaunched/in-development feature in any way?
No.
- Is the scope of this change limited to unreleased devices only? If the request is specific to an unreleased device, but the cherry-pick is not going to land in a model-specific directory, please CC a member of the SIE team to LGTM that the change will not impact any released devices.
No.
or...@google.com <or...@google.com> #22
@ch...@google.com: Could you please CC our engprod representative jinrongwu@ so she can sign off on the merge?
ji...@google.com <ji...@google.com> #23
ob...@google.com <ob...@google.com> #24
Approved for M116.
ji...@google.com <ji...@google.com> #25
jo...@google.com <jo...@google.com> #26
ji...@google.com <ji...@google.com> #27
ma...@gmail.com <ma...@gmail.com> #28
or...@google.com <or...@google.com> #29
It doesn't, that's why it's only a partial fix. It just blocks access to Mojo and chrome.send() in the first POC.
ma...@gmail.com <ma...@gmail.com> #30
bh...@google.com <bh...@google.com> #31
Also, when the chrome_url_xss_extension2.zip is extracted and opened it displays message - 'Hello! Does chrome.fileManagerPrivate exist: false' as expected.
Please refer to the screenshots attached.
ji...@google.com <ji...@google.com> #32
ap...@google.com <ap...@google.com> #33
Branch: refs/branch-heads/5845
commit 1ae7ab6ff83e8c0e82a28252ba7962e87b8ed288
Author: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Date: Thu Jul 27 04:56:00 2023
[Merge M116] webui: Filter out non-chrome scheme URLs in WebUIConfigMap
url::Origin::Create() drops filesystem: and blob: from URLs so filter
those out before using Create().
(cherry picked from commit b46c37206ab6f8df2c2ce8f23383c967360dab55)
Bug:
Change-Id: Ia8fd0d1048deaef346accd7b4cb64b448e8dc9f1
Reviewed-on:
Commit-Queue: Giovanni Ortuno Urquidi <ortuno@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1172220}
Reviewed-on:
Cr-Commit-Position: refs/branch-heads/5845@{#833}
Cr-Branched-From: 5a5dff63a4a4c63b9b18589819bebb2566c85443-refs/heads/main@{#1160321}
M chrome/browser/ash/system_extensions/system_extensions_install_manager.cc
M chrome/browser/ash/web_applications/demo_mode_app_integration_browsertest.cc
M chrome/browser/chrome_content_browser_client.cc
M content/browser/service_worker/service_worker_context_wrapper.cc
M content/browser/webui/web_ui_browsertest.cc
A content/browser/webui/webui_config_map_unittest.cc
M content/public/browser/webui_config_map.cc
M content/public/browser/webui_config_map.h
M content/public/test/scoped_web_ui_controller_factory_registration.cc
M content/public/test/scoped_web_ui_controller_factory_registration.h
M content/test/BUILD.gn
ma...@google.com <ma...@google.com> #34
Merge approved, M115
Category: Security vulnerability
ap...@google.com <ap...@google.com> #35
Branch: refs/branch-heads/5790
commit b2354da6b23520367fb7e1a972866cc75989ae13
Author: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Date: Fri Jul 28 06:21:59 2023
[Merge M115] webui: Filter out non-chrome scheme URLs in WebUIConfigMap
url::Origin::Create() drops filesystem: and blob: from URLs so filter
those out before using Create().
(cherry picked from commit b46c37206ab6f8df2c2ce8f23383c967360dab55)
(cherry picked from commit 1ae7ab6ff83e8c0e82a28252ba7962e87b8ed288)
Bug:
Change-Id: Ia8fd0d1048deaef346accd7b4cb64b448e8dc9f1
Reviewed-on:
Commit-Queue: Giovanni Ortuno Urquidi <ortuno@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Original-Original-Commit-Position: refs/heads/main@{#1172220}
Reviewed-on:
Cr-Original-Commit-Position: refs/branch-heads/5845@{#833}
Cr-Original-Branched-From: 5a5dff63a4a4c63b9b18589819bebb2566c85443-refs/heads/main@{#1160321}
Reviewed-on:
Auto-Submit: Giovanni Ortuno Urquidi <ortuno@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/branch-heads/5790@{#1864}
Cr-Branched-From: 1d71a337b1f6e707a13ae074dca1e2c34905eb9f-refs/heads/main@{#1148114}
M chrome/browser/ash/system_extensions/system_extensions_install_manager.cc
M chrome/browser/ash/web_applications/demo_mode_app_integration_browsertest.cc
M chrome/browser/chrome_content_browser_client.cc
M content/browser/service_worker/service_worker_context_wrapper.cc
M content/browser/webui/web_ui_browsertest.cc
A content/browser/webui/webui_config_map_unittest.cc
M content/public/browser/webui_config_map.cc
M content/public/browser/webui_config_map.h
M content/public/test/scoped_web_ui_controller_factory_registration.cc
M content/public/test/scoped_web_ui_controller_factory_registration.h
M content/test/BUILD.gn
jo...@google.com <jo...@google.com>
be...@google.com <be...@google.com>
lz...@google.com <lz...@google.com> #36
-
Exploitability: Explain why/why not the bug is reachable and/or exploitable
Extensions can request the browser to load a html file with the url "filesystem:chrome://file-manager/external/file", then the html file will be executed in the origin of "chrome://file-manager" and has access to private APIs.
-
Privileges and Capabilities
filesystem:chrome://file-manager/ is incorrectly treated as the same origin from chrome://file-manager, and file from "filesystem:chrome://file-manager/" loaded as WebUI.
-
Origin of fix - Is the issue already known upstream, fixed by work from a previously known or reported issue, provided by the reporter, or any other information that would be relevant toward reward eligibility
The fix is authored by a Google engineer.
-
Mitigations - Detail any regarding mitigation considerations (we're run across a few comments, such as "we considered this issue to be highly mitigated" without explanation)
The first step of mitigations is to disallow "filesystem:" to be loaded as WebUI. This is done by
https://chromium-review.googlesource.com/c/chromium/src/+/4686653 -
Severity assessment - why not higher, why not lower
High severity. Why not higher/lower: According to the guideline, this is Critical severity downgraded to High due to requiring user actions to trigger. When exploited, its severity is critical, it allows code injection that can utilize Chrome private APIs that read/write system data, communicate with system processes. The exploit requires a user action to trigger, which is to install a malicious extension.
A separate issue also reported here is that the malicious extension can read js source code of chrome://component. I am not super familiar with extension's read access. Extension code in principal runs as part of the browser, so reading chrome:// source code is not surprising, unless we do have fine-grained access control on identifying the caller extension.
ch...@google.com <ch...@google.com> #37
1. Number of CLs needed for this fix and links to them.
2. Level of complexity (High, Medium, Low - Explain)
3. Has this been merged to a stable release? beta release?
4. Overall Recommendation (Yes, No)
rz...@google.com <rz...@google.com> #38
https://crrev.com/c/4876644 - Low, simple conflict in the browser tests
- 115, 116
- Yes
gm...@google.com <gm...@google.com> #39
ap...@google.com <ap...@google.com> #40
Branch: refs/branch-heads/5735
commit a467b6dc1420e14773deedee89432942822728bd
Author: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Date: Fri Sep 22 16:57:44 2023
[M114-LTS] webui: Filter out non-chrome scheme URLs in WebUIConfigMap
M114 merge issues:
chrome/browser/ash/web_applications/demo_mode_app_integration_browsertest.cc:
Conflict around the changed lines because create_controller_func isn't declared
in 108.
url::Origin::Create() drops filesystem: and blob: from URLs so filter
those out before using Create().
(cherry picked from commit b46c37206ab6f8df2c2ce8f23383c967360dab55)
Bug:
Change-Id: Ia8fd0d1048deaef346accd7b4cb64b448e8dc9f1
Reviewed-on:
Commit-Queue: Giovanni Ortuno Urquidi <ortuno@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1172220}
Reviewed-on:
Owners-Override: Simon Hangl <simonha@google.com>
Commit-Queue: Roger Felipe Zanoni da Silva <rzanoni@google.com>
Reviewed-by: Simon Hangl <simonha@google.com>
Cr-Commit-Position: refs/branch-heads/5735@{#1606}
Cr-Branched-From: 2f562e4ddbaf79a3f3cb338b4d1bd4398d49eb67-refs/heads/main@{#1135570}
M chrome/browser/ash/system_extensions/system_extensions_install_manager.cc
M chrome/browser/ash/web_applications/demo_mode_app_integration_browsertest.cc
M chrome/browser/chrome_content_browser_client.cc
M content/browser/service_worker/service_worker_context_wrapper.cc
M content/browser/webui/web_ui_browsertest.cc
A content/browser/webui/webui_config_map_unittest.cc
M content/public/browser/webui_config_map.cc
M content/public/browser/webui_config_map.h
M content/public/test/scoped_web_ui_controller_factory_registration.cc
M content/public/test/scoped_web_ui_controller_factory_registration.h
M content/test/BUILD.gn
Description
Important: Please do NOT add any 3rd party members to this bug!
Steps to reproduce the problem:
1. Install the extension attached below on a ChromeOS device
2. Alert box running in the origin of chrome://file-manager
Problem Description:
The Files app on ChromeOS (chrome://file-manager) internally uses the JS Origin Private File System API for storing some files.
But these files can also be viewed in the browser with the filesystem: protocol. These pages have the origin of the domain they were created on (in this case chrome://file-manager) and are capable of displaying HTML with no CSP.
In this case, this leads to a quick XSS from something like a Chrome Extension. The extension can download an HTML file, read its path, and open the corresponding filesystem: URL, which will run the HTML file's code in the context of chrome://file-manager.
The extension attached in the POC uses the chrome.downloads permission to achieve this, but it should in theory be possible without the permission.
With the ability to run code on the chrome:// URL, a malicious extension could:
1. use the chrome.send API
2. import and run internal scripts like Mojo scripts from chrome://resources
3. read all local downloaded files by simply fetching their filesystem: paths
In addition to the three effects listed in the original post, the XSS can also be used to get the source code of any Chrome page, as seen in the attached image.
Reporter credit for this bug: Derin Eryilmaz.
Chrome version: 117.0.0.0 Channel: Not sure