Skip to content

Commit

Permalink
Respect nosniff on XML external entity resources
Browse files Browse the repository at this point in the history
Bug: 352038139
Change-Id: I3f5d3007bc79fcb40d0deef8177d7e9335cfb9de
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5828467
Reviewed-by: Mike West <[email protected]>
Commit-Queue: Daniel Cheng <[email protected]>
Reviewed-by: Joey Arhar <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1351056}
  • Loading branch information
zetafunction authored and Chromium LUCI CQ committed Sep 4, 2024
1 parent d11d181 commit 5c337a0
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@
#include "third_party/blink/renderer/platform/heap/garbage_collected.h"
#include "third_party/blink/renderer/platform/instrumentation/tracing/trace_event.h"
#include "third_party/blink/renderer/platform/instrumentation/use_counter.h"
#include "third_party/blink/renderer/platform/loader/allowed_by_nosniff.h"
#include "third_party/blink/renderer/platform/loader/fetch/fetch_initiator_type_names.h"
#include "third_party/blink/renderer/platform/loader/fetch/raw_resource.h"
#include "third_party/blink/renderer/platform/loader/fetch/resource_error.h"
Expand Down Expand Up @@ -655,6 +656,12 @@ static void* OpenFunc(const char* uri) {
network::mojom::RequestMode::kSameOrigin);
Resource* resource =
RawResource::FetchSynchronously(params, document->Fetcher());

if (!AllowedByNosniff::MimeTypeAsXMLExternalEntity(
document->GetExecutionContext(), resource->GetResponse())) {
return &g_global_descriptor;
}

if (!resource->ErrorOccurred()) {
data = resource->ResourceBuffer();
final_url = resource->GetResponse().CurrentRequestUrl();
Expand Down
27 changes: 25 additions & 2 deletions third_party/blink/renderer/platform/loader/allowed_by_nosniff.cc
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,8 @@ bool AllowedByNosniff::MimeTypeAsScript(UseCounter& use_counter,
}
if (!allow) {
console_logger->AddConsoleMessage(
mojom::ConsoleMessageSource::kSecurity,
mojom::ConsoleMessageLevel::kError,
mojom::blink::ConsoleMessageSource::kSecurity,
mojom::blink::ConsoleMessageLevel::kError,
"Refused to execute script from '" +
response.CurrentRequestUrl().ElidedString() +
"' because its MIME type ('" + mime_type + "') is not executable.");
Expand All @@ -228,4 +228,27 @@ bool AllowedByNosniff::MimeTypeAsScript(UseCounter& use_counter,
return allow;
}

bool AllowedByNosniff::MimeTypeAsXMLExternalEntity(
ConsoleLogger* console_logger,
const ResourceResponse& response) {
if (ParseContentTypeOptionsHeader(response.HttpHeaderField(
http_names::kXContentTypeOptions)) != kContentTypeOptionsNosniff) {
return true;
}

if (MIMETypeRegistry::IsXMLExternalEntityMIMEType(
response.HttpContentType())) {
return true;
}

console_logger->AddConsoleMessage(
mojom::blink::ConsoleMessageSource::kSecurity,
mojom::blink::ConsoleMessageLevel::kError,
"Refused to load XML external entity from '" +
response.CurrentRequestUrl().ElidedString() +
"' because its MIME type ('" + response.HttpContentType() +
"') is incorrect, and strict MIME type checking is enabled.");
return false;
}

} // namespace blink
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ class PLATFORM_EXPORT AllowedByNosniff final {
ConsoleLogger*,
const ResourceResponse&,
MimeTypeCheck mime_type_check_mode);

static bool MimeTypeAsXMLExternalEntity(ConsoleLogger*,
const ResourceResponse&);
};

} // namespace blink
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,12 @@ bool MIMETypeRegistry::IsXMLMIMEType(const String& mime_type) {
return true;
}

bool MIMETypeRegistry::IsXMLExternalEntityMIMEType(const String& mime_type) {
return EqualIgnoringASCIICase(mime_type,
"application/xml-external-parsed-entity") ||
EqualIgnoringASCIICase(mime_type, "text/xml-external-parsed-entity");
}

bool MIMETypeRegistry::IsPlainTextMIMEType(const String& mime_type) {
return mime_type.StartsWithIgnoringASCIICase("text/") &&
!(EqualIgnoringASCIICase(mime_type, "text/html") ||
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,10 @@ class PLATFORM_EXPORT MIMETypeRegistry {
// https://mimesniff.spec.whatwg.org/#xml-mime-type
static bool IsXMLMIMEType(const String& mime_type);

// Returns true if the MIME type is suitable for loading as a XML external
// entity.
static bool IsXMLExternalEntityMIMEType(const String& mime_type);

// Checks to see if a mime type is suitable for being loaded as plain text.
static bool IsPlainTextMIMEType(const String& mime_type);
};
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
CONSOLE ERROR: Refused to load XML external entity from 'http://127.0.0.1:8000/security/contentTypeOptions/resources/script-with-header.pl?mime=application/xml' because its MIME type ('application/xml') is incorrect, and strict MIME type checking is enabled.
CONSOLE ERROR: Refused to load XML external entity from 'http://127.0.0.1:8000/security/contentTypeOptions/resources/script-with-header.pl?mime=text/xml' because its MIME type ('text/xml') is incorrect, and strict MIME type checking is enabled.
CONSOLE ERROR: Refused to load XML external entity from 'http://127.0.0.1:8000/security/contentTypeOptions/resources/script-with-header.pl?mime=text/html' because its MIME type ('text/html') is incorrect, and strict MIME type checking is enabled.
CONSOLE ERROR: Refused to load XML external entity from 'http://127.0.0.1:8000/security/contentTypeOptions/resources/script-with-header.pl?mime=text/javascript' because its MIME type ('text/javascript') is incorrect, and strict MIME type checking is enabled.
CONSOLE MESSAGE: Executed script with MIME type: 'application/xml-external-parsed-entity'.
CONSOLE MESSAGE: Executed script with MIME type: 'text/xml-external-parsed-entity'.
Test nosniff when loading XML external entities.On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".PASS successfullyParsed is true
TEST COMPLETE

Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN"
"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd"
[
<!ENTITY entity_application_xml_external_parsed_entity SYSTEM "http://127.0.0.1:8000/security/contentTypeOptions/resources/script-with-header.pl?mime=application/xml-external-parsed-entity">
<!ENTITY entity_text_xml_external_parsed_entity SYSTEM "http://127.0.0.1:8000/security/contentTypeOptions/resources/script-with-header.pl?mime=text/xml-external-parsed-entity">
<!ENTITY entity_application_xml SYSTEM "http://127.0.0.1:8000/security/contentTypeOptions/resources/script-with-header.pl?mime=application/xml">
<!ENTITY entity_text_xml SYSTEM "http://127.0.0.1:8000/security/contentTypeOptions/resources/script-with-header.pl?mime=text/xml">
<!ENTITY entity_text_html SYSTEM "http://127.0.0.1:8000/security/contentTypeOptions/resources/script-with-header.pl?mime=text/html">
<!ENTITY entity_text_javascript SYSTEM "http://127.0.0.1:8000/security/contentTypeOptions/resources/script-with-header.pl?mime=text/javascript">
]>
<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
<head>
<script src="/js-test-resources/js-test.js"></script>
<script type="text/javascript"><![CDATA[
window.jsTestIsAsync = true;
description("Test nosniff when loading XML external entities.");
window.onload = () => {
finishJSTest();
}
]]></script>
<script type="text/javascript">&entity_application_xml_external_parsed_entity;</script>
<script type="text/javascript">&entity_text_xml_external_parsed_entity;</script>
<script type="text/javascript">&entity_application_xml;</script>
<script type="text/javascript">&entity_text_xml;</script>
<script type="text/javascript">&entity_text_html;</script>
<script type="text/javascript">&entity_text_javascript;</script>
</head>
</html>

0 comments on commit 5c337a0

Please sign in to comment.