Security: Privilege Escalation using extension filesystem URLs |
||||||||||||||||||
Issue descriptionThis is copied from the report in issue 835887: ---- 2. The fix[1] stopped the "noopener" trick from working for filesystem:chrome-extension: URLs, but there is another way to bypass the web_accessible_resource restriction. The URL check in ExtensionNavigationThrottle[2] relies on the output of |GetLastCommittedURL()|. A compromised renderer can commit any chrome-extension: URL through |RenderFrameHostImpl:: DidCommitSameDocumentNavigation()|. There is also an explicit check for filesystem:chrome-extension: URLs, but it doesn't apply to child frames. 1 - https://chromium.googlesource.com/chromium/src/+/1cb9c33c4afd6df591c6306511bac80ec216d463 2 - https://cs.chromium.org/chromium/src/extensions/browser/extension_navigation_throttle.cc?rcl=7e651fba081b5a1507f36a844cb33c09ed66d2f5&l=35 ---- This report bypasses work done in issue 820838, issue 821138 , and issue 821596 .
,
Apr 25 2018
The general flow of this part of the attack is that a web page in a compromised process does a window.open to about:blank, but it lies about the origin and claims it is an extension. It then does a replaceState to change the URL to be an extension URL as well, which allows the attack to embed an extension filesystem URL as a subframe. That presumably loads as an OOPIF in an extension process, with privileges. For the first step (about:blank with extension origin), RenderFrameHostImpl::CanCommitOrigin does look at this origin, but our current checks there don't catch that it's an extension origin in a web process. At the time those checks were written, --isolate-extensions hadn't launched yet and it was possible to have extension URLs/origins commit in web processes. We should make sure that the CanCommitOrigin and CanCommitURL functions in RFHI catch any attempts to commit extensions (and WebUI) in the wrong process, and kill the process instead. I have a CL in progress at https://chromium-review.googlesource.com/c/chromium/src/+/1025075. It's currently facing a problem when error pages commit in the wrong process, in which case we would kill a web process that tries to show an error page for a blocked extension URL. Nasko happens to be fixing that in https://chromium-review.googlesource.com/c/chromium/src/+/762520, but even then we'll have to make an exception for the error page process and possibly for subframes. For now, I'll probably make an exception for error pages in CanCommitURL, ensuring that the origin is unique. That will require us to check for error pages when doing the subframe filesystem URL check for extensions-- normally we allow extension filesystem URLs to be embedded as subframes of extension pages, but we won't allow that for error pages.
,
Apr 25 2018
I'm still working on the CanCommitURL CL, but it's proving to be a bit tricky given all the exceptions it needs (e.g., for GuestViews in V2 apps, for signin, for error pages). In parallel, I put together a CL that applies the extension process check in ExtensionNavigationThrottle to all frames instead of just to main frames (as alexmos@ suggested): https://chromium-review.googlesource.com/c/chromium/src/+/1028511 It seems to be working so far, so I'm writing a test for it and I think we can probably land that first. That would disrupt the chain by preventing the exploit from loading a filesystem: or blob: URL from a subframe that isn't an extension process, even if the main frame incorrectly has an extension origin. We'll still proceed with the CanCommitURL check so that the main frame won't have the incorrect extension origin, but we'll at least have something in place.
,
Apr 26 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7614790c80996d32a28218f4d1605b0908e9ddf6 commit 7614790c80996d32a28218f4d1605b0908e9ddf6 Author: Charlie Reis <creis@chromium.org> Date: Thu Apr 26 01:21:07 2018 Apply ExtensionNavigationThrottle filesystem/blob checks to all frames. BUG= 836858 Change-Id: I34333a72501129fd40b5a9aa6378c9f35f1e7fc2 Reviewed-on: https://chromium-review.googlesource.com/1028511 Reviewed-by: Devlin <rdevlin.cronin@chromium.org> Reviewed-by: Alex Moshchuk <alexmos@chromium.org> Reviewed-by: Nick Carter <nick@chromium.org> Commit-Queue: Charlie Reis <creis@chromium.org> Cr-Commit-Position: refs/heads/master@{#553867} [modify] https://crrev.com/7614790c80996d32a28218f4d1605b0908e9ddf6/chrome/browser/extensions/process_manager_browsertest.cc [modify] https://crrev.com/7614790c80996d32a28218f4d1605b0908e9ddf6/content/browser/frame_host/render_frame_host_impl.cc [modify] https://crrev.com/7614790c80996d32a28218f4d1605b0908e9ddf6/content/browser/frame_host/render_frame_host_impl.h [modify] https://crrev.com/7614790c80996d32a28218f4d1605b0908e9ddf6/content/public/test/browser_test_utils.cc [modify] https://crrev.com/7614790c80996d32a28218f4d1605b0908e9ddf6/content/public/test/browser_test_utils.h [modify] https://crrev.com/7614790c80996d32a28218f4d1605b0908e9ddf6/extensions/browser/extension_navigation_throttle.cc
,
Apr 26 2018
,
Apr 26 2018
,
Apr 26 2018
,
Apr 30 2018
govind@ / awhalley@: Requesting merge to M67. r553867 should disrupt one of the privilege escalation steps in issue 835887. It has been baking on Canary since 68.0.3410.0, and there don't appear to be new crashes from it, or new bugs about filesystem: or blob: URLs. We think it should be a safe merge and important for disrupting the sandbox escape. abdulsyed@ / awhalley@: I could request merge to M66 as well. Should we aim for M66? If so, should I let this bake in M67 beta this week before I request the merge?
,
Apr 30 2018
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
,
Apr 30 2018
govind@ - good for 67 creis@ - yep, let's get this out in beta first. Thanks!
,
Apr 30 2018
Approving merge to M67 branch 3396 based on comment #10. Please merge ASAP. Thank you.
,
Apr 30 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e2dc6347d3b22d9c2e51fa8ec9752ed4805a65eb commit e2dc6347d3b22d9c2e51fa8ec9752ed4805a65eb Author: Charlie Reis <creis@chromium.org> Date: Mon Apr 30 18:09:26 2018 Merge to M67: Apply ExtensionNavigationThrottle filesystem/blob checks to all frames. BUG= 836858 Change-Id: I34333a72501129fd40b5a9aa6378c9f35f1e7fc2 Reviewed-on: https://chromium-review.googlesource.com/1028511 Reviewed-by: Devlin <rdevlin.cronin@chromium.org> Reviewed-by: Alex Moshchuk <alexmos@chromium.org> Reviewed-by: Nick Carter <nick@chromium.org> Commit-Queue: Charlie Reis <creis@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#553867}(cherry picked from commit 7614790c80996d32a28218f4d1605b0908e9ddf6) Reviewed-on: https://chromium-review.googlesource.com/1035369 Reviewed-by: Charlie Reis <creis@chromium.org> Cr-Commit-Position: refs/branch-heads/3396@{#382} Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428} [modify] https://crrev.com/e2dc6347d3b22d9c2e51fa8ec9752ed4805a65eb/chrome/browser/extensions/process_manager_browsertest.cc [modify] https://crrev.com/e2dc6347d3b22d9c2e51fa8ec9752ed4805a65eb/content/browser/frame_host/render_frame_host_impl.cc [modify] https://crrev.com/e2dc6347d3b22d9c2e51fa8ec9752ed4805a65eb/content/browser/frame_host/render_frame_host_impl.h [modify] https://crrev.com/e2dc6347d3b22d9c2e51fa8ec9752ed4805a65eb/content/public/test/browser_test_utils.cc [modify] https://crrev.com/e2dc6347d3b22d9c2e51fa8ec9752ed4805a65eb/content/public/test/browser_test_utils.h [modify] https://crrev.com/e2dc6347d3b22d9c2e51fa8ec9752ed4805a65eb/extensions/browser/extension_navigation_throttle.cc
,
May 1 2018
Please mark security bugs as fixed as soon as the fix lands, and before requesting merges. This update is based on the merge- labels applied to this issue. Please reopen if this update was incorrect. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
May 2 2018
,
May 7 2018
Requesting to merge r553867 to M66, now that it has baked on M67 beta since May 2. This disrupts the privilege escalation used in issue 835887.
,
May 7 2018
Approving merge for M66. Branch:3359
,
May 7 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/66dd2ea1ea9eba0354a0a66718cc0755965c85f4 commit 66dd2ea1ea9eba0354a0a66718cc0755965c85f4 Author: Charlie Reis <creis@chromium.org> Date: Mon May 07 22:28:33 2018 Merge to M66: Apply ExtensionNavigationThrottle filesystem/blob checks to all frames. BUG= 836858 Change-Id: I34333a72501129fd40b5a9aa6378c9f35f1e7fc2 Reviewed-on: https://chromium-review.googlesource.com/1028511 Reviewed-by: Devlin <rdevlin.cronin@chromium.org> Reviewed-by: Alex Moshchuk <alexmos@chromium.org> Reviewed-by: Nick Carter <nick@chromium.org> Commit-Queue: Charlie Reis <creis@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#553867}(cherry picked from commit 7614790c80996d32a28218f4d1605b0908e9ddf6) Reviewed-on: https://chromium-review.googlesource.com/1048645 Reviewed-by: Charlie Reis <creis@chromium.org> Cr-Commit-Position: refs/branch-heads/3359@{#807} Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276} [modify] https://crrev.com/66dd2ea1ea9eba0354a0a66718cc0755965c85f4/chrome/browser/extensions/process_manager_browsertest.cc [modify] https://crrev.com/66dd2ea1ea9eba0354a0a66718cc0755965c85f4/content/browser/frame_host/render_frame_host_impl.cc [modify] https://crrev.com/66dd2ea1ea9eba0354a0a66718cc0755965c85f4/content/browser/frame_host/render_frame_host_impl.h [modify] https://crrev.com/66dd2ea1ea9eba0354a0a66718cc0755965c85f4/content/public/test/browser_test_utils.cc [modify] https://crrev.com/66dd2ea1ea9eba0354a0a66718cc0755965c85f4/content/public/test/browser_test_utils.h [modify] https://crrev.com/66dd2ea1ea9eba0354a0a66718cc0755965c85f4/extensions/browser/extension_navigation_throttle.cc
,
May 8 2018
The filesystem URL fix has been merged to M66 and M67, and I've filed issue 840857 to track the CanCommitURL fix. We can consider this one finished.
,
May 9 2018
,
May 10 2018
,
Jul 23
(reward-topanel as part of considering issue 835887)
,
Jul 30
,
Aug 8
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 creis@chromium.org
, Apr 25 2018