Block execution of failed 'crossorigin' <script>s.

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 48 matching lines @@
     , m_startLineNumber(WTF::OrdinalNumber::beforeFirst())
     , m_parserInserted(parserInserted)
     , m_isExternalScript(false)
     , m_alreadyStarted(alreadyStarted)
     , m_haveFiredLoad(false)
     , m_willBeParserExecuted(false)
     , m_readyToBeParserExecuted(false)
     , m_willExecuteWhenDocumentFinishedParsing(false)
     , m_forceAsync(!parserInserted)
     , m_willExecuteInOrder(false)
+    , m_asPotentiallyCORSEnabledLoad(false)
 {
     ASSERT(m_element);
     if (parserInserted && element->document().scriptableDocumentParser() && !element->document().isInDocumentWrite())
         m_startLineNumber = element->document().scriptableDocumentParser()->lineNumber();
 }
 
 ScriptLoader::~ScriptLoader()
 {
     stopLoadRequest();
 }
@@ skipping 155 matching lines @@
         m_willExecuteInOrder = true;
         contextDocument->scriptRunner()->queueScriptForExecution(this, m_resource, ScriptRunner::IN_ORDER_EXECUTION);
         m_resource->addClient(this);
     } else if (client->hasSourceAttribute()) {
         contextDocument->scriptRunner()->queueScriptForExecution(this, m_resource, ScriptRunner::ASYNC_EXECUTION);
         m_resource->addClient(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));
+        executePotentiallyCrossOriginScript(ScriptSourceCode(scriptContent(), scriptURL, position));

[abarth-chromium, 2013/11/14 16:34:48]

Does it matter that we're losing the return value here?  In particular, we do
need to return false from this function when we don't execute the script?

[sof, 2013/11/14 17:12:03]

It does matter , i.e., the prepareScript() return value indicates if the script
executes as far as I've been able to determine (which makes sense wrt
the underlying spec text.)

XMLDocumentParser is currently the only caller that checks; will test & fix
this.

[sof, 2013/11/15 08:05:23]

prepareScript() now returns 'false' if executePotentiallyCrossOriginScript()
fails.

     }
 
     return true;
 }
 
 bool ScriptLoader::fetchScript(const String& sourceUrl)
 {
     ASSERT(m_element);
 
     RefPtr<Document> elementDocument(m_element->document());
     if (!m_element->dispatchBeforeLoadEvent(sourceUrl))
         return false;
     if (!m_element->inDocument() || m_element->document() != elementDocument)
         return false;
 
     ASSERT(!m_resource);
     if (!stripLeadingAndTrailingHTMLSpaces(sourceUrl).isEmpty()) {
         FetchRequest request(ResourceRequest(elementDocument->completeURL(sourceUrl)), m_element->localName());
 
         String crossOriginMode = m_element->fastGetAttribute(HTMLNames::crossoriginAttr);
         if (!crossOriginMode.isNull()) {
             StoredCredentials allowCredentials = equalIgnoringCase(crossOriginMode, "use-credentials") ? AllowStoredCredentials : DoNotAllowStoredCredentials;
             request.setPotentiallyCrossOriginEnabled(elementDocument->securityOrigin(), allowCredentials);
+            m_asPotentiallyCORSEnabledLoad = true;
         }
         request.setCharset(scriptCharset());
 
         bool isValidScriptNonce = elementDocument->contentSecurityPolicy()->allowScriptNonce(m_element->fastGetAttribute(HTMLNames::nonceAttr));
         if (isValidScriptNonce)
             request.setContentSecurityCheck(DoNotCheckContentSecurityPolicy);
 
         m_resource = elementDocument->fetcher()->fetchScript(request);
         m_isExternalScript = true;
     }
 
-    if (m_resource) {
+    if (m_resource)
         return true;
-    }
 
     dispatchErrorEvent();
     return false;
 }
 
 bool isHTMLScriptLoader(Element* element)
 {
     return element->hasTagName(HTMLNames::scriptTag);
 }
 
@@ skipping 63 matching lines @@
     ASSERT(resource);
     if (resource->errorOccurred()) {
         dispatchErrorEvent();
     } else if (!resource->wasCanceled()) {
         executeScript(ScriptSourceCode(resource));
         dispatchLoadEvent();
     }
     resource->removeClient(this);
 }
 
+bool ScriptLoader::executePotentiallyCrossOriginScript(const ScriptSourceCode& sourceCode)

[abarth-chromium, 2013/11/14 16:34:48]

It looks like the one caller ignores the return value...

+{
+    if (sourceCode.resource()
+        && asPotentiallyCORSEnabledLoad()
+        && !m_element->document().fetcher()->canAccess(sourceCode.resource(), asPotentiallyCORSEnabledLoad())) {

[abarth-chromium, 2013/11/14 16:34:48]

It seems strange that we need to check asPotentiallyCORSEnabledLoad given that
canAccess checks "resource->options().requestOriginPolicy ==
PotentiallyCrossOriginEnabled".  Do we need this duplicated state?

[sof, 2013/11/14 17:12:03]

We don't use that resource state any longer in canAccess(), as the resource may
have been cached & wouldn't have that policy set.

+        dispatchErrorEvent();
+        return false;
+    }
+    executeScript(sourceCode);
+    return true;
+}
+
 void ScriptLoader::notifyFinished(Resource* resource)
 {
     ASSERT(!m_willBeParserExecuted);
 
     RefPtr<Document> elementDocument(m_element->document());
     RefPtr<Document> contextDocument = elementDocument->contextDocument().get();
     if (!contextDocument)
         return;
 
     // Resource possibly invokes this notifyFinished() more than
     // once because ScriptLoader doesn't unsubscribe itself from
     // Resource here and does it in execute() instead.
     // We use m_resource to check if this function is already called.
     ASSERT_UNUSED(resource, resource == m_resource);
     if (!m_resource)
         return;
-    if (!elementDocument->fetcher()->canAccess(m_resource.get())) {
+    if (!elementDocument->fetcher()->canAccess(m_resource.get(), asPotentiallyCORSEnabledLoad())) {
         dispatchErrorEvent();
         return;
     }
 
     if (m_willExecuteInOrder)
         contextDocument->scriptRunner()->notifyScriptReady(this, ScriptRunner::IN_ORDER_EXECUTION);
     else
         contextDocument->scriptRunner()->notifyScriptReady(this, ScriptRunner::ASYNC_EXECUTION);
 
     m_resource = 0;
@@ skipping 42 matching lines @@
     if (isHTMLScriptLoader(element))
         return toHTMLScriptElement(element)->loader();
 
     if (isSVGScriptLoader(element))
         return toSVGScriptElement(element)->loader();
 
     return 0;
 }
 
 }