New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 795450 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
not working at Google anymore
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug



Sign in to add a comment

Make cross-site document blocking deterministic when sniffing

Project Member Reported by creis@chromium.org, Dec 15 2017

Issue description

The document blocking logic for Site Isolation added in r522016 has a flaw where the sniffing logic can fail to confirm a response is HTML, XML, or JSON if the first read from the network is too small to sniff.  This is problematic, because it will allow the renderer to receive responses that would have otherwise been blocked if we'd read more data.  (This is not a problem for responses with a nosniff header.)

Note that this bug explains the flakiness observed in issue 794883, since the original test case there was missing a nosniff header.

This is somewhat challenging to fix in ResourceHandlers because of the complexity of buffering the response.


 

Comment 1 by creis@chromium.org, Dec 15 2017

Labels: M-64
Nick found a way to fix this in https://chromium-review.googlesource.com/c/chromium/src/+/825960 without adding a full state machine to the ResourceHandler.

We'll likely want this for M64, though it's probably ok not to have it in M63 since nosniff is still an effective way to protect resources.
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 19 2017

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

commit 4e4a85fed62e5744877dac849ba08693a417d587
Author: Nick Carter <nick@chromium.org>
Date: Tue Dec 19 07:17:15 2017

CrossSiteDocumentResourceHandler: network-independent mime type sniffing

Enhance the mime sniffers used by CrossSiteDocumentResourceHandler to expose
an "maybe" result in addition to "yes" and "no". This indeterminate result,
for example, would be returned if the first packet contained only "<". If
that is followed by "html", it would sniff as HTML; if it were followed by
"svg" it would not.

In CrossSiteDocumentResourceHandler, if we don't have a definitive answer
from the sniffers, keep buffering data until at least net::kMaxBytesToSniff
bytes arrive. Additionally, don't sniff beyond net::kMaxBytesToSniff, even if
more data is available -- this ensures determinism of the blocking logic,
regardless of how the network delivers the stream.

In CrossSiteDocumentResourceHandlerTest, modify the TestScenario so that the
response body can arrive in multiple chunks, and include an expectation
(|verdict_packet|) of which chunk will trigger the decision. Add
TestScenarios to cover indeterminate sniffing cases, and empty-response
cases.

Bug:  795450 
Change-Id: I5eaee2bb79c49db206264cc2255608152bd49a71
Reviewed-on: https://chromium-review.googlesource.com/825960
Commit-Queue: Charlie Reis <creis@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524962}
[modify] https://crrev.com/4e4a85fed62e5744877dac849ba08693a417d587/content/browser/loader/cross_site_document_resource_handler.cc
[modify] https://crrev.com/4e4a85fed62e5744877dac849ba08693a417d587/content/browser/loader/cross_site_document_resource_handler.h
[modify] https://crrev.com/4e4a85fed62e5744877dac849ba08693a417d587/content/browser/loader/cross_site_document_resource_handler_unittest.cc
[modify] https://crrev.com/4e4a85fed62e5744877dac849ba08693a417d587/content/common/cross_site_document_classifier.cc
[modify] https://crrev.com/4e4a85fed62e5744877dac849ba08693a417d587/content/common/cross_site_document_classifier.h
[modify] https://crrev.com/4e4a85fed62e5744877dac849ba08693a417d587/content/common/cross_site_document_classifier_unittest.cc
[modify] https://crrev.com/4e4a85fed62e5744877dac849ba08693a417d587/content/renderer/loader/site_isolation_stats_gatherer.cc

Comment 3 by creis@chromium.org, Dec 27 2017

Status: Fixed (was: Started)
Fixed in r524962 in 65.0.3300.0.  Thanks!
Project Member

Comment 4 by sheriffbot@chromium.org, Dec 28 2017

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Project Member

Comment 5 by sheriffbot@chromium.org, Apr 5 2018

Labels: -Restrict-View-SecurityNotify allpublic
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