Clipboard: Fix UaP in ClipboardWriter/FileReaderLoader (M88).
Make ClipboardWriter keep FileReaderLoader alive until it's done reading
a Blob, by using SelfKeepAlive<T>.
Previously, ClipboardWriter could be garbage collected unexpectedly
(ex. when the frame detaches). This could cause a use after poison
in the FileReaderLoader, where:
(1) A ClipboardWriter's FileReaderLoader starts reading the input blob
as an async task.
(2) The ClipboardWriter is destroyed (garbage collected).
(3) The FileReaderLoader completes its async task of reading the input
blob, and calls ClipboardWriter::StartWrite on the destroyed, owning
ClipboardWriter.
Additionally, add a "context destroyed" error message when a context
detaches.
TBR=mek@chromium.org
(cherry picked from commit 0f6df4faf919a4c688803e727cb12f0bacea72e4)
Bug: 1142331
Change-Id: I427cd6dc02e773b2d235d45bd9ad8935b575ff71
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2509033
Commit-Queue: Darwin Huang <huangdarwin@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#827371}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2552756
Reviewed-by: Darwin Huang <huangdarwin@chromium.org>
Cr-Commit-Position: refs/branch-heads/4324@{#209}
Cr-Branched-From: c73b5a651d37a6c4d0b8e3262cc4015a5579c6c8-refs/heads/master@{#827102}
diff --git a/third_party/blink/renderer/modules/clipboard/clipboard_promise.cc b/third_party/blink/renderer/modules/clipboard/clipboard_promise.cc
index 9b18577e..f84c352 100644
--- a/third_party/blink/renderer/modules/clipboard/clipboard_promise.cc
+++ b/third_party/blink/renderer/modules/clipboard/clipboard_promise.cc
@@ -107,7 +107,7 @@
ClipboardPromise::ClipboardPromise(ExecutionContext* context,
ScriptState* script_state)
- : ExecutionContextClient(context),
+ : ExecutionContextLifecycleObserver(context),
script_state_(script_state),
script_promise_resolver_(
MakeGarbageCollected<ScriptPromiseResolver>(script_state)),
@@ -496,13 +496,20 @@
return GetExecutionContext()->GetTaskRunner(TaskType::kUserInteraction);
}
+// ExecutionContextLifecycleObserver implementation.
+void ClipboardPromise::ContextDestroyed() {
+ script_promise_resolver_->Reject(MakeGarbageCollected<DOMException>(
+ DOMExceptionCode::kNotAllowedError, "Document detached."));
+ clipboard_writer_.Clear();
+}
+
void ClipboardPromise::Trace(Visitor* visitor) const {
visitor->Trace(script_state_);
visitor->Trace(script_promise_resolver_);
visitor->Trace(clipboard_writer_);
visitor->Trace(permission_service_);
visitor->Trace(clipboard_item_data_);
- ExecutionContextClient::Trace(visitor);
+ ExecutionContextLifecycleObserver::Trace(visitor);
}
} // namespace blink
diff --git a/third_party/blink/renderer/modules/clipboard/clipboard_promise.h b/third_party/blink/renderer/modules/clipboard/clipboard_promise.h
index ad440ce3..25a2ce01 100644
--- a/third_party/blink/renderer/modules/clipboard/clipboard_promise.h
+++ b/third_party/blink/renderer/modules/clipboard/clipboard_promise.h
@@ -27,7 +27,7 @@
class ClipboardItemOptions;
class ClipboardPromise final : public GarbageCollected<ClipboardPromise>,
- public ExecutionContextClient {
+ public ExecutionContextLifecycleObserver {
public:
// Creates promise to execute Clipboard API functions off the main thread.
static ScriptPromise CreateForRead(ExecutionContext*,
@@ -86,6 +86,9 @@
scoped_refptr<base::SingleThreadTaskRunner> GetTaskRunner();
+ // ExecutionContextLifecycleObserver
+ void ContextDestroyed() override;
+
Member<ScriptState> script_state_;
Member<ScriptPromiseResolver> script_promise_resolver_;
diff --git a/third_party/blink/renderer/modules/clipboard/clipboard_writer.cc b/third_party/blink/renderer/modules/clipboard/clipboard_writer.cc
index 2abc1e8c..cec4830 100644
--- a/third_party/blink/renderer/modules/clipboard/clipboard_writer.cc
+++ b/third_party/blink/renderer/modules/clipboard/clipboard_writer.cc
@@ -298,9 +298,12 @@
file_reading_task_runner_(promise->GetExecutionContext()->GetTaskRunner(
TaskType::kFileReading)),
system_clipboard_(system_clipboard),
- raw_system_clipboard_(raw_system_clipboard) {}
+ raw_system_clipboard_(raw_system_clipboard),
+ self_keep_alive_(PERSISTENT_FROM_HERE, this) {}
-ClipboardWriter::~ClipboardWriter() = default;
+ClipboardWriter::~ClipboardWriter() {
+ DCHECK(!file_reader_);
+}
// static
bool ClipboardWriter::IsValidType(const String& type, bool is_raw) {
@@ -330,12 +333,16 @@
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DOMArrayBuffer* array_buffer = file_reader_->ArrayBufferResult();
DCHECK(array_buffer);
+
file_reader_.reset();
+ self_keep_alive_.Clear();
StartWrite(array_buffer, clipboard_task_runner_);
}
void ClipboardWriter::DidFail(FileErrorCode error_code) {
+ file_reader_.reset();
+ self_keep_alive_.Clear();
promise_->RejectFromReadOrDecodeFailure();
}
diff --git a/third_party/blink/renderer/modules/clipboard/clipboard_writer.h b/third_party/blink/renderer/modules/clipboard/clipboard_writer.h
index 19aed56..2ca24aa3 100644
--- a/third_party/blink/renderer/modules/clipboard/clipboard_writer.h
+++ b/third_party/blink/renderer/modules/clipboard/clipboard_writer.h
@@ -9,6 +9,7 @@
#include "third_party/blink/renderer/core/fileapi/blob.h"
#include "third_party/blink/renderer/core/fileapi/file_reader_loader_client.h"
#include "third_party/blink/renderer/platform/heap/heap.h"
+#include "third_party/blink/renderer/platform/heap/self_keep_alive.h"
#include "third_party/skia/include/core/SkImage.h"
namespace blink {
@@ -27,6 +28,11 @@
// take advantage of vulnerabilities in their decoders. In
// ClipboardRawDataWriter, this decoding is skipped.
// (3) Writing the blob's decoded contents to the system clipboard.
+//
+// ClipboardWriter is owned only by itself and ClipboardPromise. It keeps
+// itself alive for the duration of FileReaderLoader's async operations using
+// SelfKeepAlive, and keeps itself alive afterwards during cross-thread
+// operations by using WrapCrossThreadPersistent.
class ClipboardWriter : public GarbageCollected<ClipboardWriter>,
public FileReaderLoaderClient {
public:
@@ -86,6 +92,10 @@
Member<SystemClipboard> system_clipboard_;
// Access to the global unsanitized system clipboard.
Member<RawSystemClipboard> raw_system_clipboard_;
+
+ // Oilpan: ClipboardWriter must remain alive until Member<T>::Clear() is
+ // called, to keep the FileReaderLoader alive and avoid unexpected UaPs.
+ SelfKeepAlive<ClipboardWriter> self_keep_alive_;
};
} // namespace blink
diff --git a/third_party/blink/web_tests/clipboard/async-clipboard/clipboard-garbage-collection-race-condition.html b/third_party/blink/web_tests/clipboard/async-clipboard/clipboard-garbage-collection-race-condition.html
new file mode 100644
index 0000000..6b813db
--- /dev/null
+++ b/third_party/blink/web_tests/clipboard/async-clipboard/clipboard-garbage-collection-race-condition.html
@@ -0,0 +1,31 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>
+ Async Clipboard write garbage collection race condition test
+</title>
+<script src="../../resources/testharness.js"></script>
+<script src="../../resources/testharnessreport.js"></script>
+<script src="../../resources/testdriver.js"></script>
+<script src="../../resources/testdriver-vendor.js"></script>
+
+<script>
+promise_test(async t => {
+ await test_driver.set_permission({name: 'clipboard-write'}, 'granted');
+
+ const blobText = new Blob(['test text'], {type: 'text/plain'});
+ const clipboardItemInput = new ClipboardItem(
+ {'text/plain' : blobText});
+
+ const promise = navigator.clipboard.write([clipboardItemInput]);
+ for (let i = 0; i < 10; ++i) {
+ // TODO(https://github.com/web-platform-tests/wpt/issues/7899): Support
+ // cross-browser GC triggers, and move this test to the Web Platform
+ // Test suite.
+ if(window.gc) {
+ window.gc();
+ }
+ await new Promise(resolve => setTimeout(resolve, 0));
+ }
+ await promise;
+}, 'Verify write clipboard avoids garbage collection race condition.');
+</script>
\ No newline at end of file
