So overall I think it's not super great that FencedFrames can load localhost/etc, but there is still other validation to prevent reading more sensitive files. I did notice that while the /etc directory is loaded, individual files still could not be read.

IsValidFencedFrameURL just validates the URL is using a valid scheme, there is further validation done on the URL outside of this method.

I'm going to tentatively set this to medium for now.

Setting milestone because of s2 severity.

Setting Priority to P1 to match Severity s2. If this is incorrect, please reset the priority. The automation bot account won't make this change again.

Thanks for the report! This does seem like a bug to me. The net::IsLocalhost() check was introduced in https://crrev.com/c/3627924 and the intent was to only allow http://localhost URLs; see issue 40224641 (and issue 40834046, where fenced frames were originally restricted to https and about:blank URLs after a similar report showed how they could navigate to an invalid scheme). So I think it needs to be strengthened to disallow file URLs (which could indeed have a host); it doesn't seem like fenced frames should be able to load them.

lbrady@ and shivanisha@ should be able to comment about which flags would currently be required for fenced frames to be enabled.

Re: headers. Generally, Fenced frames can only load content that has a Supports-Loading-Mode response header, with the exception of (1) responses that have no headers and (2) about/data/blob URLs. Looking at how file URLs load in DevTools, it looks like the reporter is right that file URLs that point to individual files do carry a couple of response headers (Content-Type and Last-Modified), while those for directories do not and hence hit the exception in (1). I've checked that this is true on Linux, Mac, and Windows. So this header check is a nice mitigating factor that would block FFs from loading individual files, though listing (possibly sensitive) directories is still not great.

I'm also curious if (1) other features might be inadvertently allowing file://localhost/etc URLs since net::IsLocalhost() usage is pretty widespread, and (2) why our normal mechanisms that disallow web content from navigating to local resources like file URLs aren't kicking in and producing a "Not allowed to load local resource" error in blink (maybe this is just due to how FFs navigations are plumbed).

nvm I missed the "existing same-site(!) processes" part Hey Chrome experts! I found this in https://chromium.googlesource.com/chromium/src/+/main/docs/process_model_and_site_isolation.md:

> Aggressive Reuse: For some cases (including on Android), Chromium will aggressively look for existing same-site processes to reuse even before reaching the process limit. Out-of-process iframes (OOPIFs) and fenced frames use this approach, such that an example.com iframe in a cross-site page will be placed in an existing example.com process (in any browsing context group), even if the process limit has not been reached. This keeps the process count lower, based on the assumption that most iframes/fenced frames are less resource demanding than top-level documents. Similarly, ServiceWorkers are generally placed in the same process as a document that is likely to rely on them.

**Does that mean this bug allows embedding process with "file://" site into the embedder process? If so, does that mean it violate Site Isolation? (if it actually allows loading file:/// into http:// context)

I'm on code health rotation this week, but I'll be able to take a look next week. The fix shouldn't be too complicated; it will need to check the scheme alongside the localhost check for that case.

Project: chromium/src
Branch: main
Author: Liam Brady <lbrady@google.com>
Link:      https://chromium-review.googlesource.com/5902121

Fenced frame: Prevent file://localhost/* files from loading.

Fenced frame: Prevent file://localhost/* files from loading.

Fenced frames are meant to be able to load the following URLs with a
default constructor:
- about:blank URLs
- https://* URLs
- http://localhost URLs

The `net::IsLocalhost()` check that is currently being used for the last
case is only checking that the URL starts with "localhost", not that
the scheme is http/https. This allows the fenced frame to load URLs like file://localhost/path/to/file. This CL tightens the check to also
check that the scheme is HTTP. The "https://localhost/*" case will
still be covered by the "https://*" check.

Bug: 368628042
Change-Id: If53573f5701c28e5674ecb524191cddc12b950fe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5902121
Reviewed-by: Shivani Sharma <shivanisha@chromium.org>
Commit-Queue: Liam Brady <lbrady@google.com>
Cr-Commit-Position: refs/heads/main@{#1368957}

lbrady: Uh oh! This issue still open and hasn't been updated in the last 14 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 Disable-Nags (case sensitive) to the Chromium Labels custom field.

Congratulations someoneverycurious! Thank you for your efforts and reporting this issue to us.

This bug has been closed for more than 14 weeks. Removing issue access restrictions.