Fire error event when script integrity check fails.

Source/core/dom/ScriptLoader.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  * Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008 Apple Inc. All rights reserved.
  * Copyright (C) 2008 Nikolas Zimmermann <zimmermann@kde.org>
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
@@ skipping 248 matching lines @@
         if (frame) {
             ScriptStreamer::startStreaming(m_pendingScript, frame->settings(), ScriptState::forMainWorld(frame));
         }
         contextDocument->scriptRunner()->queueScriptForExecution(this, ScriptRunner::ASYNC_EXECUTION);
         // Note that watchForLoad can immediately call notifyFinished.
         m_pendingScript.watchForLoad(this);
     } else {
         // Reset line numbering for nested writes.
         TextPosition position = elementDocument.isInDocumentWrite() ? TextPosition() : scriptStartPosition;
         KURL scriptURL = (!elementDocument.isInDocumentWrite() && m_parserInserted) ? elementDocument.url() : KURL();
-        executeScript(ScriptSourceCode(scriptContent(), scriptURL, position));
+        if (!executeScript(ScriptSourceCode(scriptContent(), scriptURL, position)))
+            return false;
     }
 
     return true;
 }
 
 bool ScriptLoader::fetchScript(const String& sourceUrl, FetchRequest::DeferOption defer)
 {
     ASSERT(m_element);
 
     RefPtrWillBeRawPtr<Document> elementDocument(m_element->document());
@@ skipping 30 matching lines @@
     ASSERT(element);
     return isHTMLScriptElement(*element);
 }
 
 bool isSVGScriptLoader(Element* element)
 {
     ASSERT(element);
     return isSVGScriptElement(*element);
 }
 
-void ScriptLoader::executeScript(const ScriptSourceCode& sourceCode, double* compilationFinishTime)
+bool ScriptLoader::executeScript(const ScriptSourceCode& sourceCode, double* compilationFinishTime)
 {
     ASSERT(m_alreadyStarted);
 
     if (sourceCode.isEmpty())
-        return;
+        return true;
 
     RefPtrWillBeRawPtr<Document> elementDocument(m_element->document());
     RefPtrWillBeRawPtr<Document> contextDocument = elementDocument->contextDocument().get();
     if (!contextDocument)
-        return;
+        return true;
 
     LocalFrame* frame = contextDocument->frame();
 
     const ContentSecurityPolicy* csp = elementDocument->contentSecurityPolicy();
     bool shouldBypassMainWorldCSP = (frame && frame->script().shouldBypassMainWorldCSP())
         || csp->allowScriptWithNonce(m_element->fastGetAttribute(HTMLNames::nonceAttr))
         || csp->allowScriptWithHash(sourceCode.source());
 
     if (!m_isExternalScript && (!shouldBypassMainWorldCSP && !csp->allowInlineScript(elementDocument->url(), m_startLineNumber, sourceCode.source())))
-        return;
+        return true;

[Mike West, 2015/03/23 19:49:43]

Ah, this is apparently what we do. Why wouldn't we return `false` here?

[jww, 2015/03/23 21:56:52]

So you've hit on a CSP bug that I came across writing this. I filed
https://crbug.com/469877 to track it, and we certainly should fix it, but
basically, we've been incorrectly *not* firing error events in several cases
where we should be for quite some time now. I'd like to make another CL after
this one to address it, however, as it's a whole set of tests that are unrelated
to the core SRI issue here.

[Mike West, 2015/03/24 04:51:31]

Ok. Can you add that bug to the CL description as followup work?

 
     if (m_isExternalScript) {
         ScriptResource* resource = m_resource ? m_resource.get() : sourceCode.resource();
         if (resource && !resource->mimeTypeAllowedByNosniff()) {
             contextDocument->addConsoleMessage(ConsoleMessage::create(SecurityMessageSource, ErrorMessageLevel, "Refused to execute script from '" + resource->url().elidedString() + "' because its MIME type ('" + resource->mimeType() + "') is not executable, and strict MIME type checking is enabled."));
-            return;
+            return true;

[Mike West, 2015/03/23 19:49:43]

Or here?

[jww, 2015/03/23 21:56:52]

See above.

         }
 
         if (resource && resource->mimeType().lower().startsWith("image/")) {
             contextDocument->addConsoleMessage(ConsoleMessage::create(SecurityMessageSource, ErrorMessageLevel, "Refused to execute script from '" + resource->url().elidedString() + "' because its MIME type ('" + resource->mimeType() + "') is not executable."));
             UseCounter::count(frame, UseCounter::BlockedSniffingImageToScript);
-            return;
+            return true;

[Mike West, 2015/03/23 19:49:43]

Or here?

[jww, 2015/03/23 21:56:52]

See above.

         }
     }
 
     // FIXME: Can this be moved earlier in the function?
     // Why are we ever attempting to execute scripts without a frame?
     if (!frame)
-        return;
+        return true;

[Mike West, 2015/03/23 19:49:43]

Or here? (We should probably try making this an ASSERT at some point)

[jww, 2015/03/23 21:56:52]

This is the only one I feel slightly different about and would rather not touch.
I, frankly, don't understand the implications of this, and I think you're
probably right that it would be an ASSERT, so I'd rather just leave it untouched
for now (at the very least, I don't want to tack the ASSERT change onto another
CL as I suspect there's a good chance of breakage).

[Mike West, 2015/03/24 04:51:31]

I meant making it an ASSERT in a subsequent CL, certainly. I think it'd be a
good idea for you to try to do that, but I agree that it doesn't block this CL.

 
     AccessControlStatus corsCheck = NotSharableCrossOrigin;
     if (!m_isExternalScript || (sourceCode.resource() && sourceCode.resource()->passesAccessControlCheck(&m_element->document(), m_element->document().securityOrigin())))
         corsCheck = SharableCrossOrigin;
 
     if (m_isExternalScript) {
         const KURL resourceUrl = sourceCode.resource()->resourceRequest().url();
         if (!SubresourceIntegrity::CheckSubresourceIntegrity(*m_element, sourceCode.source(), sourceCode.resource()->url(), sourceCode.resource()->mimeType(), *sourceCode.resource())) {
-            return;
+            return false;
         }
     }
 
     const bool isImportedScript = contextDocument != elementDocument;
     // http://www.whatwg.org/specs/web-apps/current-work/#execute-the-script-block step 2.3
     // with additional support for HTML imports.
     IgnoreDestructiveWriteCountIncrementer ignoreDestructiveWriteCountIncrementer(m_isExternalScript || isImportedScript ? contextDocument.get() : 0);
 
     if (isHTMLScriptLoader(m_element))
         contextDocument->pushCurrentScript(toHTMLScriptElement(m_element));
 
     // Create a script from the script element node, using the script
     // block's source and the script block's type.
     // Note: This is where the script is compiled and actually executed.
     frame->script().executeScriptInMainWorld(sourceCode, corsCheck, compilationFinishTime);
 
     if (isHTMLScriptLoader(m_element)) {
         ASSERT(contextDocument->currentScript() == m_element);
         contextDocument->popCurrentScript();
     }
+
+    return true;
 }
 
 void ScriptLoader::execute()
 {
     ASSERT(!m_willBeParserExecuted);
     ASSERT(m_pendingScript.resource());
     bool errorOccurred = false;
     ScriptSourceCode source = m_pendingScript.getSource(KURL(), errorOccurred);
     RefPtrWillBeRawPtr<Element> element = m_pendingScript.releaseElementAndClear();
     ALLOW_UNUSED_LOCAL(element);
     if (errorOccurred) {
         dispatchErrorEvent();
     } else if (!m_resource->wasCanceled()) {
-        executeScript(source);
-        dispatchLoadEvent();
+        if (executeScript(source))
+            dispatchLoadEvent();
+        else
+            dispatchErrorEvent();
     }
     m_resource = 0;
 }
 
 void ScriptLoader::notifyFinished(Resource* resource)
 {
     ASSERT(!m_willBeParserExecuted);
 
     RefPtrWillBeRawPtr<Document> elementDocument(m_element->document());
     RefPtrWillBeRawPtr<Document> contextDocument = elementDocument->contextDocument().get();
@@ skipping 64 matching lines @@
     if (isHTMLScriptLoader(element))
         return toHTMLScriptElement(element)->loader();
 
     if (isSVGScriptLoader(element))
         return toSVGScriptElement(element)->loader();
 
     return 0;
 }
 
 } // namespace blink