[Backport] Follow-up CVE-2017-5010

Make CSSFontFace::setLoadStatus post a task

Setting the load status to LOADED results in the promise associated with the FontFace "loaded" property being resolved synchronously. The promise is resolved with a loaded FontFace object. This could result in script being executed in a forbidden scope.

Posting a task when the status is ERROR/LOADED prevents the promise from being resolved inside a forbidden scope and matches the spec ("When the load operation completes, successfully or not, queue a task to run the following steps synchronously", see https://drafts.csswg.org/css-font-loading/#font-face-load).

BUG=663476

Review-Url: https://codereview.chromium.org/2610593002
Cr-Commit-Position: refs/heads/master@{#443596}

Change-Id: I740fc24d7d53b7683e6927b7abf34b358aa5122d
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

2 files changed, 42 insertions, 12 deletions

diff --git a/chromium/third_party/WebKit/Source/core/css/FontFace.cpp b/chromium/third_party/WebKit/Source/core/css/FontFace.cpp
index 6f908043004..c3fb2e658ab 100644
--- a/chromium/third_party/WebKit/Source/core/css/FontFace.cpp
+++ b/chromium/third_party/WebKit/Source/core/css/FontFace.cpp
@@ -59,6 +59,9 @@
 #include "core/frame/UseCounter.h"
 #include "platform/FontFamilyNames.h"
 #include "platform/SharedBuffer.h"
+#include "public/platform/WebTaskRunner.h"
+#include "public/platform/WebFrameScheduler.h"
+#include "public/platform/Platform.h"
 
 namespace blink {
 
@@ -330,23 +333,46 @@ void FontFace::setLoadStatus(LoadStatus status)
 {
     m_status = status;
     ASSERT(m_status != Error || m_error);
-
+    // When promises are resolved with 'thenables', instead of the object being
+    // returned directly, the 'then' method is executed (the resolver tries to
+    // resolve the thenable). This can lead to synchronous script execution, so we
+    // post a task. This does not apply to promise rejection (i.e. a thenable
+    // would be returned as is).
     if (m_status == Loaded || m_status == Error) {
         if (m_loadedProperty) {
-            if (m_status == Loaded)
-                m_loadedProperty->resolve(this);
-            else
+            if (m_status == Loaded) {
+                getTaskRunner()->postTask(
+                  BLINK_FROM_HERE, WTF::bind(&LoadedProperty::resolve<FontFace*>,
+                  m_loadedProperty,
+                  this));
+            } else
                 m_loadedProperty->reject(m_error.get());
         }
 
-        WillBeHeapVector<RefPtrWillBeMember<LoadFontCallback>> callbacks;
-        m_callbacks.swap(callbacks);
-        for (size_t i = 0; i < callbacks.size(); ++i) {
-            if (m_status == Loaded)
-                callbacks[i]->notifyLoaded(this);
-            else
-                callbacks[i]->notifyError(this);
-        }
+        getTaskRunner()->postTask(
+            BLINK_FROM_HERE,
+            WTF::bind(&FontFace::runCallbacks, this));
+    }
+}
+
+WebTaskRunner* FontFace::getTaskRunner() {
+     ExecutionContext* context = executionContext();
+     if (context && context->isDocument()) {
+         LocalFrame* frame = static_cast<Document*>(context)->frame();
+         if (frame)
+             return frame->frameScheduler()->loadingTaskRunner();
+     }
+     return Platform::current()->currentThread()->taskRunner();
+}
+
+void FontFace::runCallbacks() {
+    WillBeHeapVector<RefPtrWillBeMember<LoadFontCallback>> callbacks;
+    m_callbacks.swap(callbacks);
+    for (size_t i = 0; i < callbacks.size(); ++i) {
+        if (m_status == Loaded)
+            callbacks[i]->notifyLoaded(this);
+        else
+            callbacks[i]->notifyError(this);
     }
 }
 
diff --git a/chromium/third_party/WebKit/Source/core/css/FontFace.h b/chromium/third_party/WebKit/Source/core/css/FontFace.h
index ce45e44282c..7d116435062 100644
--- a/chromium/third_party/WebKit/Source/core/css/FontFace.h
+++ b/chromium/third_party/WebKit/Source/core/css/FontFace.h
@@ -55,6 +55,7 @@ class FontFaceDescriptors;
 class StringOrArrayBufferOrArrayBufferView;
 class StylePropertySet;
 class StyleRuleFontFace;
+class WebTaskRunner;
 
 class FontFace : public RefCountedWillBeGarbageCollectedFinalized<FontFace>, public ScriptWrappable, public ActiveDOMObject {
     DEFINE_WRAPPERTYPEINFO();
@@ -128,6 +129,9 @@ private:
     bool setFamilyValue(const CSSValue&);
     void loadInternal(ExecutionContext*);
     ScriptPromise fontStatusPromise(ScriptState*);
+    WebTaskRunner* getTaskRunner();
+    void runCallbacks();
+
 
     using LoadedProperty = ScriptPromiseProperty<RawPtrWillBeMember<FontFace>, RawPtrWillBeMember<FontFace>, Member<DOMException>>;