Skip to content

Commit

Permalink
fix: UAP in ImageBitmapLoader/FileReaderLoader (#18563)
Browse files Browse the repository at this point in the history
  • Loading branch information
miniak authored and codebytere committed Jun 4, 2019
1 parent cbc0e46 commit b35e35f
Show file tree
Hide file tree
Showing 2 changed files with 135 additions and 0 deletions.
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 {}

0 comments on commit b35e35f

Please sign in to comment.