Skip to content

GitLab

T
tangled
Commit 65f4db51 authored by yhirano's avatar yhirano Committed by Commit bot
Browse files
Options

Fix a crash of SRI when used with web workers 

SubresourceIntegrity::CheckSubresourceIntegrity expects a non-null Document
which doesn't exist in workers. This CL replaces Document with
ExecutionContext.

BUG=678515
R=jochen@chromium.org

Review-Url: https://codereview.chromium.org/2614813004
Cr-Commit-Position: refs/heads/master@{#442800}
parent b554c05a
Hide whitespace changes
Inline Side-by-side
Showing with 79 additions and 28 deletions
third_party/WebKit/LayoutTests/http/tests/security/subresourceIntegrity/resources/subresource-integrity-fetch-worker.js 0 → 100644
View file @ 65f4db51
importScripts('/resources/testharness.js');
console.log('hehehe');
const url = '../call-success.js';
const integrity = 'sha256-B0/62fJSJFrdjEFR9ba04m/D+LHQ+zG6PGcaR0Trpxg=';
promise_test(() => {
return fetch(url).then(res => res.text()).then(text => {
assert_equals(text, 'success();\n');
});
}, 'No integrity');
promise_test(() => {
return fetch(url, {integrity: integrity}).then(res => {
return res.text();
}).then(text => {
assert_equals(text, 'success();\n');
});
}, 'Good integrity');
promise_test(() => {
return fetch(url, {integrity: 'sha256-deadbeaf'}).then(res => {
assert_unreached('the integrity check should fail');
}, () => {
// The integrity check should fail.
});
}, 'Bad integrity');
done();
third_party/WebKit/LayoutTests/http/tests/security/subresourceIntegrity/subresource-integrity-fetch-worker.html 0 → 100644
View file @ 65f4db51
<!DOCTYPE html>
<html>
<head>
<title>Tests integrity enforcement on fetch() on Web Worker</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
</head>
<body>
<script>
const SCRIPT = 'resources/subresource-integrity-fetch-worker.js';
fetch_tests_from_worker(new Worker(SCRIPT));
console.log('hello');
</script>
</body>
</html>
third_party/WebKit/Source/core/frame/SubresourceIntegrity.cpp
View file @ 65f4db51
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include "core/HTMLNames.h" #include "core/HTMLNames.h"
#include "core/dom/Document.h" #include "core/dom/Document.h"
#include "core/dom/Element.h" #include "core/dom/Element.h"
#include "core/dom/ExecutionContext.h"
#include "core/fetch/Resource.h" #include "core/fetch/Resource.h"
#include "core/frame/UseCounter.h" #include "core/frame/UseCounter.h"
#include "core/inspector/ConsoleMessage.h" #include "core/inspector/ConsoleMessage.h"
...@@ -38,8 +39,9 @@ static bool isValueCharacter(UChar c) { ...@@ -38,8 +39,9 @@ static bool isValueCharacter(UChar c) {
return c >= 0x21 && c <= 0x7e; return c >= 0x21 && c <= 0x7e;
} }
static void logErrorToConsole(const String& message, Document& document) { static void logErrorToConsole(const String& message,
document.addConsoleMessage(ConsoleMessage::create( ExecutionContext& executionContext) {
executionContext.addConsoleMessage(ConsoleMessage::create(
SecurityMessageSource, ErrorMessageLevel, message)); SecurityMessageSource, ErrorMessageLevel, message));
} }
...@@ -156,18 +158,18 @@ bool SubresourceIntegrity::CheckSubresourceIntegrity( ...@@ -156,18 +158,18 @@ bool SubresourceIntegrity::CheckSubresourceIntegrity(
const char* content, const char* content,
size_t size, size_t size,
const KURL& resourceUrl, const KURL& resourceUrl,
Document& document, ExecutionContext& executionContext,
String& errorMessage) { String& errorMessage) {
IntegrityMetadataSet metadataSet; IntegrityMetadataSet metadataSet;
IntegrityParseResult integrityParseResult = IntegrityParseResult integrityParseResult = parseIntegrityAttribute(
parseIntegrityAttribute(integrityMetadata, metadataSet, &document); integrityMetadata, metadataSet, &executionContext);
// On failed parsing, there's no need to log an error here, as // On failed parsing, there's no need to log an error here, as
// parseIntegrityAttribute() will output an appropriate console message. // parseIntegrityAttribute() will output an appropriate console message.
if (integrityParseResult != IntegrityParseValidResult) if (integrityParseResult != IntegrityParseValidResult)
return true; return true;
return CheckSubresourceIntegrity(metadataSet, content, size, resourceUrl, return CheckSubresourceIntegrity(metadataSet, content, size, resourceUrl,
document, errorMessage); executionContext, errorMessage);
} }
bool SubresourceIntegrity::CheckSubresourceIntegrity( bool SubresourceIntegrity::CheckSubresourceIntegrity(
...@@ -175,7 +177,7 @@ bool SubresourceIntegrity::CheckSubresourceIntegrity( ...@@ -175,7 +177,7 @@ bool SubresourceIntegrity::CheckSubresourceIntegrity(
const char* content, const char* content,
size_t size, size_t size,
const KURL& resourceUrl, const KURL& resourceUrl,
Document& document, ExecutionContext& executionContext,
String& errorMessage) { String& errorMessage) {
if (!metadataSet.size()) if (!metadataSet.size())
return true; return true;
...@@ -202,7 +204,7 @@ bool SubresourceIntegrity::CheckSubresourceIntegrity( ...@@ -202,7 +204,7 @@ bool SubresourceIntegrity::CheckSubresourceIntegrity(
hashVector.size()); hashVector.size());
if (DigestsEqual(digest, convertedHashVector)) { if (DigestsEqual(digest, convertedHashVector)) {
UseCounter::count(document, UseCounter::count(&executionContext,
UseCounter::SRIElementWithMatchingIntegrityAttribute); UseCounter::SRIElementWithMatchingIntegrityAttribute);
return true; return true;
} }
...@@ -226,7 +228,7 @@ bool SubresourceIntegrity::CheckSubresourceIntegrity( ...@@ -226,7 +228,7 @@ bool SubresourceIntegrity::CheckSubresourceIntegrity(
"There was an error computing an integrity value for resource '" + "There was an error computing an integrity value for resource '" +
resourceUrl.elidedString() + "'. The resource has been blocked."; resourceUrl.elidedString() + "'. The resource has been blocked.";
} }
UseCounter::count(document, UseCounter::count(&executionContext,
UseCounter::SRIElementWithNonMatchingIntegrityAttribute); UseCounter::SRIElementWithNonMatchingIntegrityAttribute);
return false; return false;
} }
...@@ -317,9 +319,10 @@ SubresourceIntegrity::parseIntegrityAttribute( ...@@ -317,9 +319,10 @@ SubresourceIntegrity::parseIntegrityAttribute(
} }
SubresourceIntegrity::IntegrityParseResult SubresourceIntegrity::IntegrityParseResult
SubresourceIntegrity::parseIntegrityAttribute(const WTF::String& attribute, SubresourceIntegrity::parseIntegrityAttribute(
IntegrityMetadataSet& metadataSet, const WTF::String& attribute,
Document* document) { IntegrityMetadataSet& metadataSet,
ExecutionContext* executionContext) {
Vector<UChar> characters; Vector<UChar> characters;
attribute.stripWhiteSpace().appendTo(characters); attribute.stripWhiteSpace().appendTo(characters);
const UChar* position = characters.data(); const UChar* position = characters.data();
...@@ -351,13 +354,14 @@ SubresourceIntegrity::parseIntegrityAttribute(const WTF::String& attribute, ...@@ -351,13 +354,14 @@ SubresourceIntegrity::parseIntegrityAttribute(const WTF::String& attribute,
// Unknown hash algorithms are treated as if they're not present, // Unknown hash algorithms are treated as if they're not present,
// and thus are not marked as an error, they're just skipped. // and thus are not marked as an error, they're just skipped.
skipUntil<UChar, isASCIISpace>(position, end); skipUntil<UChar, isASCIISpace>(position, end);
if (document) { if (executionContext) {
logErrorToConsole("Error parsing 'integrity' attribute ('" + attribute + logErrorToConsole("Error parsing 'integrity' attribute ('" + attribute +
"'). The specified hash algorithm must be one of " "'). The specified hash algorithm must be one of "
"'sha256', 'sha384', or 'sha512'.", "'sha256', 'sha384', or 'sha512'.",
*document); *executionContext);
UseCounter::count( UseCounter::count(
*document, UseCounter::SRIElementWithUnparsableIntegrityAttribute); executionContext,
UseCounter::SRIElementWithUnparsableIntegrityAttribute);
} }
continue; continue;
} }
...@@ -365,14 +369,15 @@ SubresourceIntegrity::parseIntegrityAttribute(const WTF::String& attribute, ...@@ -365,14 +369,15 @@ SubresourceIntegrity::parseIntegrityAttribute(const WTF::String& attribute,
if (parseResult == AlgorithmUnparsable) { if (parseResult == AlgorithmUnparsable) {
error = true; error = true;
skipUntil<UChar, isASCIISpace>(position, end); skipUntil<UChar, isASCIISpace>(position, end);
if (document) { if (executionContext) {
logErrorToConsole("Error parsing 'integrity' attribute ('" + attribute + logErrorToConsole("Error parsing 'integrity' attribute ('" + attribute +
"'). The hash algorithm must be one of 'sha256', " "'). The hash algorithm must be one of 'sha256', "
"'sha384', or 'sha512', followed by a '-' " "'sha384', or 'sha512', followed by a '-' "
"character.", "character.",
*document); *executionContext);
UseCounter::count( UseCounter::count(
*document, UseCounter::SRIElementWithUnparsableIntegrityAttribute); executionContext,
UseCounter::SRIElementWithUnparsableIntegrityAttribute);
} }
continue; continue;
} }
...@@ -382,13 +387,14 @@ SubresourceIntegrity::parseIntegrityAttribute(const WTF::String& attribute, ...@@ -382,13 +387,14 @@ SubresourceIntegrity::parseIntegrityAttribute(const WTF::String& attribute,
if (!parseDigest(position, currentIntegrityEnd, digest)) { if (!parseDigest(position, currentIntegrityEnd, digest)) {
error = true; error = true;
skipUntil<UChar, isASCIISpace>(position, end); skipUntil<UChar, isASCIISpace>(position, end);
if (document) { if (executionContext) {
logErrorToConsole( logErrorToConsole(
"Error parsing 'integrity' attribute ('" + attribute + "Error parsing 'integrity' attribute ('" + attribute +
"'). The digest must be a valid, base64-encoded value.", "'). The digest must be a valid, base64-encoded value.",
*document); *executionContext);
UseCounter::count( UseCounter::count(
*document, UseCounter::SRIElementWithUnparsableIntegrityAttribute); executionContext,
UseCounter::SRIElementWithUnparsableIntegrityAttribute);
} }
continue; continue;
} }
...@@ -400,11 +406,12 @@ SubresourceIntegrity::parseIntegrityAttribute(const WTF::String& attribute, ...@@ -400,11 +406,12 @@ SubresourceIntegrity::parseIntegrityAttribute(const WTF::String& attribute,
if (skipExactly<UChar>(position, end, '?')) { if (skipExactly<UChar>(position, end, '?')) {
const UChar* begin = position; const UChar* begin = position;
skipWhile<UChar, isValueCharacter>(position, end); skipWhile<UChar, isValueCharacter>(position, end);
if (begin != position && document) if (begin != position && executionContext) {
logErrorToConsole( logErrorToConsole(
"Ignoring unrecogized 'integrity' attribute option '" + "Ignoring unrecogized 'integrity' attribute option '" +
String(begin, position - begin) + "'.", String(begin, position - begin) + "'.",
*document); *executionContext);
}
} }
IntegrityMetadata integrityMetadata(digest, algorithm); IntegrityMetadata integrityMetadata(digest, algorithm);
......
third_party/WebKit/Source/core/frame/SubresourceIntegrity.h
View file @ 65f4db51
...@@ -14,8 +14,8 @@ ...@@ -14,8 +14,8 @@
namespace blink { namespace blink {
class Document;
class Element; class Element;
class ExecutionContext;
class KURL; class KURL;
class Resource; class Resource;
...@@ -46,13 +46,13 @@ class CORE_EXPORT SubresourceIntegrity { ...@@ -46,13 +46,13 @@ class CORE_EXPORT SubresourceIntegrity {
const char*, const char*,
size_t, size_t,
const KURL& resourceUrl, const KURL& resourceUrl,
Document&, ExecutionContext&,
WTF::String&); WTF::String&);
static bool CheckSubresourceIntegrity(const IntegrityMetadataSet&, static bool CheckSubresourceIntegrity(const IntegrityMetadataSet&,
const char*, const char*,
size_t, size_t,
const KURL& resourceUrl, const KURL& resourceUrl,
Document&, ExecutionContext&,
WTF::String&); WTF::String&);
// The IntegrityMetadataSet arguments are out parameters which contain the // The IntegrityMetadataSet arguments are out parameters which contain the
...@@ -63,7 +63,7 @@ class CORE_EXPORT SubresourceIntegrity { ...@@ -63,7 +63,7 @@ class CORE_EXPORT SubresourceIntegrity {
static IntegrityParseResult parseIntegrityAttribute( static IntegrityParseResult parseIntegrityAttribute(
const WTF::String& attribute, const WTF::String& attribute,
IntegrityMetadataSet&, IntegrityMetadataSet&,
Document*); ExecutionContext*);
private: private:
friend class SubresourceIntegrityTest; friend class SubresourceIntegrityTest;
......
third_party/WebKit/Source/modules/fetch/FetchManager.cpp
View file @ 65f4db51
...@@ -215,7 +215,7 @@ class FetchManager::Loader final ...@@ -215,7 +215,7 @@ class FetchManager::Loader final
if (r == WebDataConsumerHandle::Done) { if (r == WebDataConsumerHandle::Done) {
if (SubresourceIntegrity::CheckSubresourceIntegrity( if (SubresourceIntegrity::CheckSubresourceIntegrity(
m_integrityMetadata, m_buffer.data(), m_buffer.size(), m_url, m_integrityMetadata, m_buffer.data(), m_buffer.size(), m_url,
*m_loader->document(), errorMessage)) { *m_loader->executionContext(), errorMessage)) {
m_updater->update( m_updater->update(
new FormDataBytesConsumer(m_buffer.data(), m_buffer.size())); new FormDataBytesConsumer(m_buffer.data(), m_buffer.size()));
m_loader->m_resolver->resolve(m_response); m_loader->m_resolver->resolve(m_response);
...@@ -272,6 +272,7 @@ class FetchManager::Loader final ...@@ -272,6 +272,7 @@ class FetchManager::Loader final
void failed(const String& message); void failed(const String& message);
void notifyFinished(); void notifyFinished();
Document* document() const; Document* document() const;
ExecutionContext* executionContext() { return m_executionContext; }
void loadSucceeded(); void loadSucceeded();
Member<FetchManager> m_fetchManager; Member<FetchManager> m_fetchManager;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment