Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: UAP in ImageBitmapLoader/FileReaderLoader #18563

Merged
merged 4 commits into from Jun 4, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions patches/common/chromium/.patches
Expand Up @@ -105,3 +105,4 @@ restore_live_region_changed_events_for_processing_by_jaws_focus_mode.patch
enable_quic_proxies_for_https_urls.patch
fix_svg_crash_for_v0_distribution_into_foreignobject.patch
filesystem_harden_against_overflows_of_operationid_a_bit_better.patch
fix_uap_in_imagebitmaploader_filereaderloader.patch
@@ -0,0 +1,134 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Marijn Kruisselbrink <mek@chromium.org>
Date: Thu, 13 Dec 2018 17:09:55 +0000
Subject: Fix UAP in ImageBitmapLoader/FileReaderLoader

FileReaderLoader stores its client as a raw pointer, so in cases like
ImageBitmapLoader where the FileReaderLoaderClient really is garbage
collected we have to make sure to destroy the FileReaderLoader when
the ExecutionContext that owns it is destroyed.

Bug: 913970
Change-Id: I40b02115367cf7bf5bbbbb8e9b57874d2510f861
Reviewed-on: https://chromium-review.googlesource.com/c/1374511
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616342}

diff --git a/third_party/blink/renderer/core/imagebitmap/image_bitmap_factories.cc b/third_party/blink/renderer/core/imagebitmap/image_bitmap_factories.cc
index 076b9beeaa6675c5e858a1c7f5a321ab57c606fb..40898b704891c3723574d3afa4aebf72f5cdce5c 100644
--- a/third_party/blink/renderer/core/imagebitmap/image_bitmap_factories.cc
+++ b/third_party/blink/renderer/core/imagebitmap/image_bitmap_factories.cc
@@ -238,27 +238,31 @@ void ImageBitmapFactories::DidFinishLoading(ImageBitmapLoader* loader) {
pending_loaders_.erase(loader);
}

+void ImageBitmapFactories::Trace(blink::Visitor* visitor) {
+ visitor->Trace(pending_loaders_);
+ Supplement<LocalDOMWindow>::Trace(visitor);
+ Supplement<WorkerGlobalScope>::Trace(visitor);
+}
+
ImageBitmapFactories::ImageBitmapLoader::ImageBitmapLoader(
ImageBitmapFactories& factory,
base::Optional<IntRect> crop_rect,
ScriptState* script_state,
const ImageBitmapOptions& options)
- : loader_(
+ : ContextLifecycleObserver(ExecutionContext::From(script_state)),
+ loader_(
FileReaderLoader::Create(FileReaderLoader::kReadAsArrayBuffer, this)),
factory_(&factory),
resolver_(ScriptPromiseResolver::Create(script_state)),
crop_rect_(crop_rect),
options_(options) {}

-void ImageBitmapFactories::ImageBitmapLoader::LoadBlobAsync(
- Blob* blob) {
+void ImageBitmapFactories::ImageBitmapLoader::LoadBlobAsync(Blob* blob) {
loader_->Start(blob->GetBlobDataHandle());
}

-void ImageBitmapFactories::Trace(blink::Visitor* visitor) {
- visitor->Trace(pending_loaders_);
- Supplement<LocalDOMWindow>::Trace(visitor);
- Supplement<WorkerGlobalScope>::Trace(visitor);
+ImageBitmapFactories::ImageBitmapLoader::~ImageBitmapLoader() {
+ DCHECK(!loader_);
}

void ImageBitmapFactories::ImageBitmapLoader::RejectPromise(
@@ -277,11 +281,20 @@ void ImageBitmapFactories::ImageBitmapLoader::RejectPromise(
default:
NOTREACHED();
}
+ loader_.reset();
factory_->DidFinishLoading(this);
}

+void ImageBitmapFactories::ImageBitmapLoader::ContextDestroyed(
+ ExecutionContext*) {
+ if (loader_)
+ factory_->DidFinishLoading(this);
+ loader_.reset();
+}
+
void ImageBitmapFactories::ImageBitmapLoader::DidFinishLoading() {
DOMArrayBuffer* array_buffer = loader_->ArrayBufferResult();
+ loader_.reset();
if (!array_buffer) {
RejectPromise(kAllocationFailureImageBitmapRejectionReason);
return;
@@ -359,6 +372,7 @@ void ImageBitmapFactories::ImageBitmapLoader::ResolvePromiseOnOriginalThread(
}

void ImageBitmapFactories::ImageBitmapLoader::Trace(blink::Visitor* visitor) {
+ ContextLifecycleObserver::Trace(visitor);
visitor->Trace(factory_);
visitor->Trace(resolver_);
}
diff --git a/third_party/blink/renderer/core/imagebitmap/image_bitmap_factories.h b/third_party/blink/renderer/core/imagebitmap/image_bitmap_factories.h
index 25bdf1ffd1e85a05ab4ee46aaa81c61294fd7d1a..726012a20f7250ea1166ec03875ebaa10f352a49 100644
--- a/third_party/blink/renderer/core/imagebitmap/image_bitmap_factories.h
+++ b/third_party/blink/renderer/core/imagebitmap/image_bitmap_factories.h
@@ -36,6 +36,7 @@
#include "third_party/blink/renderer/bindings/core/v8/image_bitmap_source.h"
#include "third_party/blink/renderer/bindings/core/v8/script_promise.h"
#include "third_party/blink/renderer/bindings/core/v8/script_promise_resolver.h"
+#include "third_party/blink/renderer/core/dom/context_lifecycle_observer.h"
#include "third_party/blink/renderer/core/fileapi/file_reader_loader.h"
#include "third_party/blink/renderer/core/fileapi/file_reader_loader_client.h"
#include "third_party/blink/renderer/core/frame/local_dom_window.h"
@@ -103,7 +104,10 @@ class ImageBitmapFactories final
private:
class ImageBitmapLoader final
: public GarbageCollectedFinalized<ImageBitmapLoader>,
+ public ContextLifecycleObserver,
public FileReaderLoaderClient {
+ USING_GARBAGE_COLLECTED_MIXIN(ImageBitmapLoader);
+
public:
static ImageBitmapLoader* Create(ImageBitmapFactories& factory,
base::Optional<IntRect> crop_rect,
@@ -115,9 +119,9 @@ class ImageBitmapFactories final
void LoadBlobAsync(Blob*);
ScriptPromise Promise() { return resolver_->Promise(); }

- void Trace(blink::Visitor*);
+ void Trace(blink::Visitor*) override;

- ~ImageBitmapLoader() override = default;
+ ~ImageBitmapLoader() override;

private:
ImageBitmapLoader(ImageBitmapFactories&,
@@ -140,6 +144,9 @@ class ImageBitmapFactories final
const String& color_space_conversion_option);
void ResolvePromiseOnOriginalThread(sk_sp<SkImage>);

+ // ContextLifecycleObserver
+ void ContextDestroyed(ExecutionContext*) override;
+
// FileReaderLoaderClient
void DidStartLoading() override {}
void DidReceiveData() override {}