Skip to content

Commit

Permalink
In CrossSiteDocumentResourceHandler, sniff for parser-breakers.
Browse files Browse the repository at this point in the history 
kScriptBreakingPrefixes contains prefixes that are conventionally used to
prevent a JSON response from becoming a valid Javascript program (an attack
vector known as XSSI). The presence of such a prefix is a strong signal
that the resource is meant to be consumed only by the fetch API or
XMLHttpRequest, and is meant to be protected from use in non-CORS, cross-
origin contexts like <script>, <img>, etc.

Block any matching non-CORS cross-site responses, even if they include
the nosniff header, and regardless of their server-provided MIME type.
Ignoring the nosniff header in this case allows JSON sent as (e.g.)
application/javascript to be blocked, so long as we can detect a
strong signal that it's illegal or invalid Javascript.

Bug: 795476
Change-Id: I8d8b283777d467bbe9d685ecadca090a0c09f458
Reviewed-on: https://chromium-review.googlesource.com/835003
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525608}
  • Loading branch information
@nick-chromium
nick-chromium authored and Commit Bot committed Dec 21, 2017
1 parent 2403a17 commit b569e4a
Show file tree
Hide file tree
Showing 18 changed files with 364 additions and 74 deletions.
48 changes: 34 additions & 14 deletions content/browser/loader/cross_site_document_blocking_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,7 @@ class CrossSiteDocumentBlockingBaseTest : public ContentBrowserTest {
} else if (base::MatchPattern(resource_name, "*.txt")) {
bucket = "Plain";
} else {
EXPECT_FALSE(should_be_blocked);
bucket = "Other";
bucket = "Others";
}

// Determine the appropriate histograms, including a start and end action
Expand All @@ -97,6 +96,11 @@ class CrossSiteDocumentBlockingBaseTest : public ContentBrowserTest {
base::HistogramTester::CountsMap expected_counts;
std::string base = "SiteIsolation.XSD.Browser";
expected_counts[base + ".Action"] = 2;
if ((base::MatchPattern(resource_name, "*prefixed*") ||
bucket == "Others") &&
should_be_blocked) {
expected_counts[base + ".BlockedForParserBreaker"] = 1;
}
if (should_be_sniffed)
expected_counts[base + ".BytesReadForSniffing"] = 1;
if (should_be_blocked) {
Expand Down Expand Up @@ -154,13 +158,28 @@ IN_PROC_BROWSER_TEST_F(CrossSiteDocumentBlockingTest, BlockDocuments) {

// The following are files under content/test/data/site_isolation. All
// should be disallowed for cross site XHR under the document blocking policy.
// valid.* - Correctly labeled HTML/XML/JSON files.
// *.txt - Plain text that sniffs as HTML, XML, or JSON.
// htmlN_dtd.* - Various HTML templates to test.
const char* blocked_resources[] = {
"valid.html", "valid.xml", "valid.json", "html.txt",
"xml.txt", "json.txt", "comment_valid.html", "html4_dtd.html",
"html4_dtd.txt", "html5_dtd.html", "html5_dtd.txt"};
// valid.* - Correctly labeled HTML/XML/JSON files.
// *.txt - Plain text that sniffs as HTML, XML, or JSON.
// htmlN_dtd.* - Various HTML templates to test.
// json-prefixed* - parser-breaking prefixes
const char* blocked_resources[] = {"valid.html",
"valid.xml",
"valid.json",
"html.txt",
"xml.txt",
"json.txt",
"comment_valid.html",
"html4_dtd.html",
"html4_dtd.txt",
"html5_dtd.html",
"html5_dtd.txt",
"json.js",
"json-prefixed-1.js",
"json-prefixed-2.js",
"json-prefixed-3.js",
"json-prefixed-4.js",
"nosniff.json.js",
"nosniff.json-prefixed.js"};
for (const char* resource : blocked_resources) {
SCOPED_TRACE(base::StringPrintf("... while testing page: %s", resource));
base::HistogramTester histograms;
Expand Down Expand Up @@ -196,10 +215,12 @@ IN_PROC_BROWSER_TEST_F(CrossSiteDocumentBlockingTest, BlockDocuments) {
// *js.* - JavaScript mislabeled as a document.
// jsonp.* - JSONP (i.e., script) mislabeled as a document.
// img.* - Contents that won't match the document label.
// valid.* - Correctly labeled responses of non-document types.
const char* sniff_allowed_resources[] = {
"js.html", "comment_js.html", "js.xml", "js.json", "js.txt",
"jsonp.html", "jsonp.xml", "jsonp.json", "jsonp.txt", "img.html",
"img.xml", "img.json", "img.txt"};
"js.html", "comment_js.html", "js.xml", "js.json",
"js.txt", "jsonp.html", "jsonp.xml", "jsonp.json",
"jsonp.txt", "img.html", "img.xml", "img.json",
"img.txt", "valid.js", "json-list.js", "nosniff.json-list.js"};
for (const char* resource : sniff_allowed_resources) {
SCOPED_TRACE(base::StringPrintf("... while testing page: %s", resource));
base::HistogramTester histograms;
Expand All @@ -215,9 +236,8 @@ IN_PROC_BROWSER_TEST_F(CrossSiteDocumentBlockingTest, BlockDocuments) {

// These files should be allowed for XHR under the document blocking policy.
// cors.* - Correctly labeled documents with valid CORS headers.
// valid.* - Correctly labeled responses of non-document types.
const char* allowed_resources[] = {"cors.html", "cors.xml", "cors.json",
"cors.txt", "valid.js"};
"cors.txt"};
for (const char* resource : allowed_resources) {
SCOPED_TRACE(base::StringPrintf("... while testing page: %s", resource));
base::HistogramTester histograms;
Expand Down
74 changes: 57 additions & 17 deletions content/browser/loader/cross_site_document_resource_handler.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

#include "content/browser/loader/cross_site_document_resource_handler.h"

#include <algorithm>
#include <string.h>
#include <memory>
#include <string>
#include <utility>

Expand Down Expand Up @@ -207,9 +207,8 @@ void CrossSiteDocumentResourceHandler::ResumeOnWillRead(
// This is only done when we suspect the response should be blocked.
//
// Make it as big as the downstream handler's buffer to make it easy to copy
// over in one operation. This will be large, since the MIME sniffing
// handler is downstream. Technically we could use a smaller buffer if
// |needs_sniffing_| is false, but there's no need for the extra complexity.
// over in one operation. This will be large, since the MIME sniffing handler
// is downstream.
DCHECK_GE(*buf_size, net::kMaxBytesToSniff);
local_buffer_ =
base::MakeRefCounted<net::IOBuffer>(static_cast<size_t>(*buf_size));
Expand All @@ -235,11 +234,13 @@ void CrossSiteDocumentResourceHandler::OnReadCompleted(
// response is empty and complete), or copy the sniffed data to the next
// handler's buffer and resume the response without blocking.
if (should_block_based_on_headers_ && !allow_based_on_sniffing_) {
bool found_parser_breaker = false;
auto confirmed_blockable = CrossSiteDocumentClassifier::kNo;
if (!needs_sniffing_) {
// TODO(creis): Also consider the MIME type confirmed if |bytes_read| is
// too small to do sniffing, or restructure to allow buffering enough.
// For now, responses with small initial reads may be allowed through.
// If sniffing is impossible (e.g., because this is a range request), or
// if sniffing is disabled due to a nosniff header AND the server returned
// a protected mime type, then we have enough information to block
// immediately.
confirmed_blockable = CrossSiteDocumentClassifier::kYes;
} else if (bytes_read == 0) {
// We haven't blocked the response yet (because previous reads yielded a
Expand All @@ -261,7 +262,8 @@ void CrossSiteDocumentResourceHandler::OnReadCompleted(
std::min(local_buffer_bytes_read_, net::kMaxBytesToSniff);
base::StringPiece data(local_buffer_->data(), bytes_to_sniff);

// Confirm whether the data is HTML, XML, or JSON.
// If the server returned a protected mime type, sniff the response to
// confirm it.
if (canonical_mime_type_ == CROSS_SITE_DOCUMENT_MIME_TYPE_HTML) {
confirmed_blockable = CrossSiteDocumentClassifier::SniffForHTML(data);
} else if (canonical_mime_type_ == CROSS_SITE_DOCUMENT_MIME_TYPE_XML) {
Expand All @@ -274,6 +276,18 @@ void CrossSiteDocumentResourceHandler::OnReadCompleted(
confirmed_blockable = SniffForHtmlXmlOrJson(data);
}

// Additionally, on all mime types (including _OTHERS), look for
// Javascript parser breakers. These are affirmative patterns that
// indicate this resource should only be consumed by XHR/fetch (and we've
// already verified that this response isn't a permissable cross-origin
// XHR/fetch).
if (confirmed_blockable != CrossSiteDocumentClassifier::kYes) {
auto result =
CrossSiteDocumentClassifier::SniffForFetchOnlyResource(data);
found_parser_breaker = (result == CrossSiteDocumentClassifier::kYes);
confirmed_blockable = std::max(confirmed_blockable, result);
}

// If sniffing didn't yield a conclusive response, and we haven't read too
// many bytes yet, buffer up some more data.
if (confirmed_blockable == CrossSiteDocumentClassifier::kMaybe &&
Expand Down Expand Up @@ -313,6 +327,11 @@ void CrossSiteDocumentResourceHandler::OnReadCompleted(
UMA_HISTOGRAM_ENUMERATION("SiteIsolation.XSD.Browser.Blocked",
resource_type,
content::RESOURCE_TYPE_LAST_TYPE);
if (found_parser_breaker) {
UMA_HISTOGRAM_ENUMERATION(
"SiteIsolation.XSD.Browser.BlockedForParserBreaker", resource_type,
content::RESOURCE_TYPE_LAST_TYPE);
}
switch (canonical_mime_type_) {
case CROSS_SITE_DOCUMENT_MIME_TYPE_HTML:
UMA_HISTOGRAM_ENUMERATION("SiteIsolation.XSD.Browser.Blocked.HTML",
Expand All @@ -334,6 +353,11 @@ void CrossSiteDocumentResourceHandler::OnReadCompleted(
resource_type,
content::RESOURCE_TYPE_LAST_TYPE);
break;
case CROSS_SITE_DOCUMENT_MIME_TYPE_OTHERS:
UMA_HISTOGRAM_ENUMERATION("SiteIsolation.XSD.Browser.Blocked.Others",
resource_type,
content::RESOURCE_TYPE_LAST_TYPE);
break;
default:
NOTREACHED();
}
Expand Down Expand Up @@ -432,14 +456,14 @@ bool CrossSiteDocumentResourceHandler::ShouldBlockBasedOnHeaders(
return false;
}

// Look up MIME type. If it doesn't claim to be a blockable type (i.e., HTML,
// XML, JSON, or plain text), don't block it.
// Look up MIME type. Even if it doesn't claim to be a blockable type (i.e.,
// HTML, XML, JSON, or plain text), it may still fail the checks during the
// SniffForFetchOnlyResource() phase.
//
// TODO(nick): What if the mime type is omitted? Should that be treated the
// same as text/plain? https://crbug.com/795971
canonical_mime_type_ = CrossSiteDocumentClassifier::GetCanonicalMimeType(
response->head.mime_type);
if (canonical_mime_type_ == CROSS_SITE_DOCUMENT_MIME_TYPE_OTHERS)
return false;

// Treat a missing initiator as an empty origin to be safe, though we don't
// expect this to happen. Unfortunately, this requires a copy.
Expand Down Expand Up @@ -495,16 +519,32 @@ bool CrossSiteDocumentResourceHandler::ShouldBlockBasedOnHeaders(
// sniff the contents to confirm the MIME type, to avoid blocking incorrectly
// labeled JavaScript, JSONP, etc files.
//
// Note: only sniff if there isn't a nosniff header, and if it is not a range
// request. Range requests would let an attacker bypass blocking by
// requesting a range that fails to sniff as a protected type.
// Note: if there is a nosniff header, it means we should honor the response
// mime type without trying to confirm it.
std::string nosniff_header;
response->head.headers->GetNormalizedHeader("x-content-type-options",
&nosniff_header);
bool has_nosniff_header =
base::LowerCaseEqualsASCII(nosniff_header, "nosniff");

// If this is an HTTP range request, sniffing isn't possible.
std::string range_header;
response->head.headers->GetNormalizedHeader("content-range", &range_header);
needs_sniffing_ = !base::LowerCaseEqualsASCII(nosniff_header, "nosniff") &&
range_header.empty();
bool has_range_header = !range_header.empty();

// If this is a partial response, sniffing is not possible, so allow the
// response if it's not a protected mime type.
if (has_range_header &&
canonical_mime_type_ == CROSS_SITE_DOCUMENT_MIME_TYPE_OTHERS) {
return false;
}

// We need to sniff unprotected mime types (e.g. for parser breakers), and
// unless the nosniff header is set, we also need to sniff protected mime
// types to verify that they're not mislabeled.
needs_sniffing_ =
(canonical_mime_type_ == CROSS_SITE_DOCUMENT_MIME_TYPE_OTHERS) ||
!(has_range_header || has_nosniff_header);

return true;
}
Expand Down
Loading

0 comments on commit b569e4a

Please sign in to comment.