Extensions: don't auto-grant chrome:// urls with <all_urls> to component extensions |
||||
Issue descriptionTL;DR: component extensions should request "chrome://*/*" if they want to script chrome pages. A recent exploit used the fact the pdf extension requested <all_urls> to script chrome pages. This is bad. Currently, if an extension requests <all_urls>, we grant it all viable schemes it might have access to. For component extensions, this includes chrome://*, since component extensions can script chrome:// pages (and sometimes need to, as in the case of chromevox). While we can't get rid of the ability to script chrome:// pages, we can ensure that they aren't included by default in <all_urls>, and require that a component extension that really wants to do this does so by including chrome://*/* explicitly. This would eliminate the chance of a component extension "accidentally" having access to chrome:// pages if they don't need it. This also forces component extensions to (hopefully) think about which chrome hosts they may need, leading to more specific requests (e.g., chrome://resources/*). Restricting this because of its relation to issue 821138 , but it doesn't itself represent a security vulnerability (hence Security_Impact-None).
,
Mar 30 2018
Friendly ping: dbertoni@, can you take a look?
,
May 11 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ba355f657a607c74f0de82ad925a4dc1a7c9a95b commit ba355f657a607c74f0de82ad925a4dc1a7c9a95b Author: David Bertoni <dbertoni@chromium.org> Date: Fri May 11 19:04:26 2018 Move kChromeVoxExtensionId from chrome/common/extensions/ to extensions/common/ so it's available there. The constant will be needed for https://chromium-review.googlesource.com/c/chromium/src/+/993753. Bug: 821858 Change-Id: I5caf0ee476c67c5fea276bcc92f7207a8fe24c18 Reviewed-on: https://chromium-review.googlesource.com/1050733 Reviewed-by: David Tseng <dtseng@chromium.org> Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org> Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org> Commit-Queue: David Bertoni <dbertoni@chromium.org> Cr-Commit-Position: refs/heads/master@{#557968} [modify] https://crrev.com/ba355f657a607c74f0de82ad925a4dc1a7c9a95b/chrome/browser/chromeos/accessibility/accessibility_manager.cc [modify] https://crrev.com/ba355f657a607c74f0de82ad925a4dc1a7c9a95b/chrome/browser/chromeos/accessibility/chromevox_panel.cc [modify] https://crrev.com/ba355f657a607c74f0de82ad925a4dc1a7c9a95b/chrome/browser/chromeos/accessibility/spoken_feedback_event_rewriter_delegate.cc [modify] https://crrev.com/ba355f657a607c74f0de82ad925a4dc1a7c9a95b/chrome/browser/ui/webui/settings/chromeos/accessibility_handler.cc [modify] https://crrev.com/ba355f657a607c74f0de82ad925a4dc1a7c9a95b/chrome/common/extensions/chrome_extensions_client.cc [modify] https://crrev.com/ba355f657a607c74f0de82ad925a4dc1a7c9a95b/chrome/common/extensions/extension_constants.cc [modify] https://crrev.com/ba355f657a607c74f0de82ad925a4dc1a7c9a95b/chrome/common/extensions/extension_constants.h [modify] https://crrev.com/ba355f657a607c74f0de82ad925a4dc1a7c9a95b/extensions/common/constants.cc [modify] https://crrev.com/ba355f657a607c74f0de82ad925a4dc1a7c9a95b/extensions/common/constants.h
,
May 24 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/036330515a086ef631564e254f587d58466af717 commit 036330515a086ef631564e254f587d58466af717 Author: David Bertoni <dbertoni@chromium.org> Date: Thu May 24 23:30:38 2018 [Extensions] Prevent <all_urls> pattern from matching chrome:// scheme The chrome:// scheme is sensitive enough that any extension that can have permission to run on a chrome:// page should have to specially request access to that page. Prevent <all_urls> from implicitly matching the chrome:// scheme. Bug: 821858 Change-Id: Ie321749cd93ee4b51e89f501a9a25088d3df84c4 Reviewed-on: https://chromium-review.googlesource.com/993753 Commit-Queue: David Bertoni <dbertoni@chromium.org> Reviewed-by: Devlin <rdevlin.cronin@chromium.org> Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org> Cr-Commit-Position: refs/heads/master@{#561688} [modify] https://crrev.com/036330515a086ef631564e254f587d58466af717/chrome/browser/extensions/all_urls_apitest.cc [modify] https://crrev.com/036330515a086ef631564e254f587d58466af717/chrome/common/extensions/permissions/permissions_data_unittest.cc [modify] https://crrev.com/036330515a086ef631564e254f587d58466af717/extensions/common/BUILD.gn [add] https://crrev.com/036330515a086ef631564e254f587d58466af717/extensions/common/component_extension_url_pattern_unittest.cc [modify] https://crrev.com/036330515a086ef631564e254f587d58466af717/extensions/common/manifest_handlers/content_scripts_handler.cc [modify] https://crrev.com/036330515a086ef631564e254f587d58466af717/extensions/common/manifest_handlers/permissions_parser.cc [modify] https://crrev.com/036330515a086ef631564e254f587d58466af717/extensions/common/permissions/permissions_data.cc [modify] https://crrev.com/036330515a086ef631564e254f587d58466af717/extensions/common/permissions/permissions_data.h
,
May 24 2018
,
May 25 2018
Thank you, dbertoni@! As FYI, there may be some fallout from this (that dbertoni@ and I discussed beforehand, and agreed was acceptable given the benefit) regarding extensions that relied either on chrome:// urls, or urls of arbitrary schemes that weren't included in the kValidSchemes in URLPattern [1]. We looked for any clients using this in practice, but couldn't find any from a cursory inspection. If any show up, we'll work with them to fix the issue. [1] https://chromium.googlesource.com/chromium/src/+/901ea08ea95759b78d836f6da98d319a2680e9e9/extensions/common/url_pattern.cc#30
,
May 25 2018
,
Aug 31
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 |
||||
Comment 1 by rdevlin....@chromium.org
, Mar 16 2018Owner: dbertoni@chromium.org
Status: Assigned (was: Untriaged)