From ea586408d219c9da8de412dcf8b2223fac4c4013 Mon Sep 17 00:00:00 2001 From: Michal Klocek Date: Fri, 13 Jan 2017 09:36:41 -0800 Subject: [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 --- .../WebKit/Source/core/css/FontFace.cpp | 50 ++++++++++++++++------ .../third_party/WebKit/Source/core/css/FontFace.h | 4 ++ 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, + m_loadedProperty, + this)); + } else m_loadedProperty->reject(m_error.get()); } - WillBeHeapVector> 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(context)->frame(); + if (frame) + return frame->frameScheduler()->loadingTaskRunner(); + } + return Platform::current()->currentThread()->taskRunner(); +} + +void FontFace::runCallbacks() { + WillBeHeapVector> 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, 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, Member>; -- cgit v1.2.3