Setting medium severity, since the known use for privilege escalation has been disrupted in  issue 836858 , but there's a good chance this can be used to cause other damage.

I have a CL in progress in https://chromium-review.googlesource.com/c/chromium/src/+/1025075.  It has some dependencies on Nasko's error page isolation CL (https://chromium-review.googlesource.com/c/chromium/src/+/762520), which is currently causing some regressions.  We'll see if we can get them resolved.

Friendly ping. :) This is blocking the full resolution of a Critical, so it'd be nice to nail it down.

I've revived the CL in progress and I'm working through the remaining issues.

https://chromium-review.googlesource.com/c/chromium/src/+/1025075 is back up for review, and should work whether error page isolation is enabled or not.

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

commit 2c8441547e38709d7a5c804b23375fccd0fed622
Author: Charlie Reis <creis@chromium.org>
Date: Wed Jun 13 01:41:00 2018

Make ExtensionSet::GetExtensionOrApp{ID}ByURL support filesystem URLs.

Filesystem and blob URLs don't have a chrome-extension:// scheme
according to GURL, but do according to url::Origin.

BUG= 851503 ,  840857 

Change-Id: Ia5e4cf5f9c3d04dff07d4f31da6d774d574af653
Reviewed-on: https://chromium-review.googlesource.com/1093402
Commit-Queue: Charlie Reis <creis@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566677}
[modify] https://crrev.com/2c8441547e38709d7a5c804b23375fccd0fed622/extensions/common/extension_set.cc
[modify] https://crrev.com/2c8441547e38709d7a5c804b23375fccd0fed622/extensions/common/extension_set_unittest.cc

Quick update: we're trying to wrap up one last question on https://chromium-review.googlesource.com/c/chromium/src/+/1025075, and I'm aiming to land it today.

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

commit e2c2c49f75dc103e7f49c0f3fcb7f0283ee12801
Author: Charlie Reis <creis@chromium.org>
Date: Fri Jun 15 21:34:04 2018

Do not allow extension origins or URLs to commit in web processes.

This CL revives some of the checks from r512959 to ensure that most
extension URLs only commit in the correct extension processes.
There are several exceptions that must be accounted for.

BUG= 770239 ,  840857 

Change-Id: Id3dd2a7814041186d4de6f61e2dee440939b57d9
Reviewed-on: https://chromium-review.googlesource.com/1025075
Commit-Queue: Charlie Reis <creis@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567799}
[modify] https://crrev.com/e2c2c49f75dc103e7f49c0f3fcb7f0283ee12801/chrome/browser/chrome_security_exploit_browsertest.cc
[modify] https://crrev.com/e2c2c49f75dc103e7f49c0f3fcb7f0283ee12801/chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc
[modify] https://crrev.com/e2c2c49f75dc103e7f49c0f3fcb7f0283ee12801/chrome/browser/extensions/process_management_browsertest.cc
[modify] https://crrev.com/e2c2c49f75dc103e7f49c0f3fcb7f0283ee12801/content/browser/frame_host/navigation_request.cc
[modify] https://crrev.com/e2c2c49f75dc103e7f49c0f3fcb7f0283ee12801/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/e2c2c49f75dc103e7f49c0f3fcb7f0283ee12801/content/browser/security_exploit_browsertest.cc
[modify] https://crrev.com/e2c2c49f75dc103e7f49c0f3fcb7f0283ee12801/content/public/test/browser_test_utils.cc
[modify] https://crrev.com/e2c2c49f75dc103e7f49c0f3fcb7f0283ee12801/content/public/test/browser_test_utils.h
[modify] https://crrev.com/e2c2c49f75dc103e7f49c0f3fcb7f0283ee12801/extensions/browser/url_request_util.cc
[modify] https://crrev.com/e2c2c49f75dc103e7f49c0f3fcb7f0283ee12801/extensions/browser/url_request_util.h

Tentatively marking fixed as of r567799.  We'll see if that triggers any incorrect kills over the weekend.

I'm not sure if we'll want to merge this or not, given the risk of regressions from new renderer kills.  If we don't see any after a week, though, maybe it's worth considering for M68.

The NextAction date has arrived: 2018-06-18

Note that I have found a few renderer kills due to the new check.  It's very low volume so I don't think we need to revert the check yet, but we should diagnose what's causing them, and I don't plan to merge the CL to M68 as a result.

Looks like there have been 5 CanCommitURL kills since the CL landed in 69.0.3462.0 (plus 1 unrelated CWS kill which we've seen before):
https://crash.corp.google.com/browse?q=expanded_custom_data.ChromeCrashProto.magic_signature_1.name%3D%27%5BRenderer+kill+1%5D+content%3A%3ARenderFrameHostImpl%3A%3AValidateDidCommitParams%27+AND+product.Version%3E%3D%2769.0.3462.0%27#samplereports

All 5 of the new ones are extension URLs that load visibly in tabs (though only some are web accessible resources).  They're all bad_message_reason 1 (RFH_CAN_COMMIT_URL_BLOCKED), and thus CanCommitURL rather than CanCommitOrigin.

I've ruled out cases that an extension is uninstalled (since we will only kill the process if we find the enabled extension object, and since all the crash reports indicate the user did have the extension itself installed).

I thought it might happen if an extension process crashes, then a new process gets created for it elsewhere (e.g., background page or another tab), and then an earlier sad tab gets reloaded into the old RenderProcessHost.  If a new process replaced the old one in the extension logic's ProcessMap, and the old one came back when reloading the sad tab, it might have explained it.  However, that kill doesn't seem to happen when I try it in practice.

I'm trying a diagnostic CL in https://chromium-review.googlesource.com/c/chromium/src/+/1108968 to see if we can learn anything from the SiteInstance's site URL and process's origin lock, to see if the extension URL is committing in an unexpected process.  I would expect all extension URLs to commit in their own SiteInstance and in a process without an origin lock (with or without SitePerProcess).

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

commit 1d3d1e2ed534aae0eb96f5c0f77c992f9f1f28d2
Author: Charlie Reis <creis@chromium.org>
Date: Thu Jun 21 00:46:11 2018

Add crash keys to help diagnose CanCommitURL renderer kills.

Logging the SiteInstance's site URL will help clarify why the (already
logged) URL was blocked.

This also logs default values for origin lock and site isolation mode
in case those strings are empty.

BUG= 840857 

Change-Id: I260bdf5960146a2c8fcab7d3daa56ba9bfa0d05f
Reviewed-on: https://chromium-review.googlesource.com/1108968
Commit-Queue: Charlie Reis <creis@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569104}
[modify] https://crrev.com/1d3d1e2ed534aae0eb96f5c0f77c992f9f1f28d2/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/1d3d1e2ed534aae0eb96f5c0f77c992f9f1f28d2/content/browser/renderer_host/render_process_host_impl.cc

The diagnostic CL landed in r569104 (69.0.3468.0), and we've seen 3 kills since then (still not very high volume):

crash/cf4258d80cf5651d 
crash/1efa2a94872d631a 
crash/fe437589005075be 

We are indeed killing processes whose SiteInstance's site URL matches the extension that is being committed.  

These recent cases all have the process locked to that extension as well, which is a bit unexpected (since we don't lock extension processes to origin in general).  That was only true for 1 of 4 of the previous extension kills, so I'm not sure what triggers it.

I had one theory that this could be happening in cases where multiple processes exist for an extension due to incognito split mode (and indeed, the klbibkeccnjlkjkiokjodocebajanakg extension seen in two of these kills does use that mode).  process_map.h calls that out as a tricky case, but I think we're handling it correctly already-- both processes return true from ProcessMap::Contains(extension_id, process_id), and I can't repro the kill that way.

For now, I think it's best to make an exception and allow extension URLs to commit in extension processes, even if they aren't correctly registered in the ProcessMap.  That's less pressing from a security perspective because extensions are allowed to share processes with each other already, and because we really wanted to prevent web processes from committing these URLs.  I have a CL to add an exception like that here, to try to avoid the kills:
https://chromium-review.googlesource.com/c/chromium/src/+/1120840

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

commit fc00fd072ce760c057df0c06f9eaff86f1736e6d
Author: Charlie Reis <creis@chromium.org>
Date: Fri Jun 29 21:07:45 2018

Avoid renderer kills for extension URLs in the wrong extension process.

We should track down the cause, but the intent of the restriction is to
prevent web processes from committing extension URLs, so make an
exception for now.

BUG= 840857 

Change-Id: I5259f870140896400eb076b870981750a9ff73cd
Reviewed-on: https://chromium-review.googlesource.com/1120840
Commit-Queue: Charlie Reis <creis@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571626}
[modify] https://crrev.com/fc00fd072ce760c057df0c06f9eaff86f1736e6d/chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc

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