Chrome · WebShare
CVE-2026-12437
UAF in WebShare
Overview
Critical
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Changed Functions
| Function | Change | Notes |
|---|---|---|
ifchrome/browser/webshare/win/fake_data_transfer_manager.cc |
modified | |
output_stream_chrome/browser/webshare/win/fake_data_writer_factory.cc |
modified | |
ifchrome/browser/webshare/win/fake_data_writer_factory.cc |
modified | |
fake_data_writer_factorychrome/browser/webshare/win/scoped_share_operation_fake_components.cc |
modified | |
TEST_Fchrome/browser/webshare/win/share_operation_unittest.cc |
modified |
Files Changed
chrome/browser/webshare/win/fake_data_transfer_manager.ccchrome/browser/webshare/win/fake_data_writer_factory.ccchrome/browser/webshare/win/fake_data_writer_factory.hchrome/browser/webshare/win/fake_storage_file_statics.ccchrome/browser/webshare/win/scoped_share_operation_fake_components.ccchrome/browser/webshare/win/scoped_share_operation_fake_components.hchrome/browser/webshare/win/share_operation_unittest.cc
Patch
From d04b3a27cd55a11ea63b90747be6bc757d87ceb1 Mon Sep 17 00:00:00 2001 From: Hoch Hochkeppel <[email protected]> Date: Sat, 06 Jun 2026 09:39:48 -0700 Subject: [PATCH] WebShare: Windows test improvements (leaky mocks) Small improvements made to the mock classes to better mimic Windows by more aggressively releasing references. Previously, several of the associated tests were leaking some of these mocks, and thus skipping some of their destructor safety checks. This change also required updating a couple tests that started to fail due to having these checks now enforced. Found as part of investigating: Bug: 516496659 Change-Id: Ia757a9de762f254ff61a2870eecef0568db29fcd Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7905903 Commit-Queue: Kurt Catti-Schmidt <[email protected]> Reviewed-by: Kurt Catti-Schmidt <[email protected]> Reviewed-by: Daniel Murphy <[email protected]> Cr-Commit-Position: refs/heads/main@{#1642811} --- diff --git a/chrome/browser/webshare/win/fake_data_transfer_manager.cc b/chrome/browser/webshare/win/fake_data_transfer_manager.cc index a0105d4..99c03a3 100644 --- a/chrome/browser/webshare/win/fake_data_transfer_manager.cc +++ b/chrome/browser/webshare/win/fake_data_transfer_manager.cc @@ -283,7 +283,13 @@ // IDataRequestDeferral IFACEMETHODIMP Complete() final { + if (!data_request_) { + ADD_FAILURE() + << "Complete called on IDataRequestDeferral more than once"; + return E_FAIL; + } data_request_->RunPostDataRequestedCallbackImpl(); + data_request_ = nullptr; return S_OK; } diff --git a/chrome/browser/webshare/win/fake_data_writer_factory.cc b/chrome/browser/webshare/win/fake_data_writer_factory.cc index d72ea08d..1a39b97 100644 --- a/chrome/browser/webshare/win/fake_data_writer_factory.cc +++ b/chrome/browser/webshare/win/fake_data_writer_factory.cc @@ -39,8 +39,11 @@ : public RuntimeClass<RuntimeClassFlags<Microsoft::WRL::WinRtClassicComMix>, IDataWriter> { public: - explicit FakeDataWriter(IOutputStream* output_stream) - : output_stream_(output_stream) {} + explicit FakeDataWriter(bool check_for_unflushed_writer_destroyed, + IOutputStream* output_stream) + : check_for_unflushed_writer_destroyed_( + check_for_unflushed_writer_destroyed), + output_stream_(output_stream) {} FakeDataWriter(const FakeDataWriter&) = delete; FakeDataWriter& operator=(const FakeDataWriter&) = delete; ~FakeDataWriter() final { @@ -48,8 +51,10 @@ << "FakeDataWriter destroyed with data pending storage."; EXPECT_FALSE(store_async_in_progress_) << "FakeDataWriter destroyed while store operation is in progress."; - EXPECT_TRUE(flush_called_) - << "FakeDataWriter destroyed without calling FlushAsync."; + if (check_for_unflushed_writer_destroyed_) { + EXPECT_TRUE(flush_called_) + << "FakeDataWriter destroyed without calling FlushAsync."; + } } // IDataWriter @@ -232,6 +237,7 @@ } private: + bool check_for_unflushed_writer_destroyed_; ComPtr<IBuffer> buffer_; bool flush_called_ = false; ComPtr<IOutputStream> output_stream_; @@ -251,7 +257,8 @@ ADD_FAILURE() << "CreateDataWriter called with null output_stream."; return E_INVALIDARG; } - auto fake_data_writer = Make<FakeDataWriter>(output_stream); + auto fake_data_writer = Make<FakeDataWriter>( + check_for_unflushed_writer_destroyed_, output_stream); HRESULT hr = fake_data_writer->QueryInterface(IID_PPV_ARGS(data_writer)); if (FAILED(hr)) { EXPECT_HRESULT_SUCCEEDED(hr); @@ -260,4 +267,8 @@ return S_OK; } +void FakeDataWriterFactory::SetCheckForUnflushedWriterDestroyed(bool check) { + check_for_unflushed_writer_destroyed_ = check; +} + } // namespace webshare diff --git a/chrome/browser/webshare/win/fake_data_writer_factory.h b/chrome/browser/webshare/win/fake_data_writer_factory.h index dadbdf5..91b58354 100644 --- a/chrome/browser/webshare/win/fake_data_writer_factory.h +++ b/chrome/browser/webshare/win/fake_data_writer_factory.h @@ -26,6 +26,16 @@ IFACEMETHODIMP CreateDataWriter( ABI::Windows::Storage::Streams::IOutputStream* outputStream, ABI::Windows::Storage::Streams::IDataWriter** data_writer) final; + + // By default any IDataWriters created by this factory will check if they are + // destroyed without having FlushAsync called on them, as this should not + // happen for normal operations. If a test needs to allow for this (for + // example, to test an error case where the operation is abandoned) it should + // call this to disable/re-enable that check. + void SetCheckForUnflushedWriterDestroyed(bool check); + + private: + bool check_for_unflushed_writer_destroyed_ = true; }; } // namespace webshare diff --git a/chrome/browser/webshare/win/fake_storage_file_statics.cc b/chrome/browser/webshare/win/fake_storage_file_statics.cc index e0b7bd7..275082fa 100644 --- a/chrome/browser/webshare/win/fake_storage_file_statics.cc +++ b/chrome/browser/webshare/win/fake_storage_file_statics.cc @@ -212,8 +212,19 @@ fake_iasync_operation->CompleteWithResults(random_access_stream); })); + // The IStreamedFileDataRequestedHandler should not be invoked multiple + // times, so we clear it here after invocation, mimicking Windows behavior. + // + // Technically opening the same IStorageFile multiple times is allowed by + // Windows (it caches the results of the first call to the + // IStreamedFileDataRequestedHandler), but we shouldn't have a practical + // reason to ever do so. If the need arises this test mock can be updated to + // support that behavior, but more likely the calling code should be + // adjusted to only try to open the file once. + ASSERT_TRUE(streamed_file_data_requested_handler_); ASSERT_HRESULT_SUCCEEDED( streamed_file_data_requested_handler_->Invoke(output_stream.Get())); + streamed_file_data_requested_handler_ = nullptr; } std::string display_name_with_extension_; diff --git a/chrome/browser/webshare/win/scoped_share_operation_fake_components.cc b/chrome/browser/webshare/win/scoped_share_operation_fake_components.cc index 3bfd344..e80d305 100644 --- a/chrome/browser/webshare/win/scoped_share_operation_fake_components.cc +++ b/chrome/browser/webshare/win/scoped_share_operation_fake_components.cc @@ -91,4 +91,9 @@ return scoped_fake_data_transfer_manager_interop_.instance(); } +FakeDataWriterFactory& +ScopedShareOperationFakeComponents::fake_data_writer_factory() { + return *fake_data_writer_factory_.Get(); +} + } // namespace webshare diff --git a/chrome/browser/webshare/win/scoped_share_operation_fake_components.h b/chrome/browser/webshare/win/scoped_share_operation_fake_components.h index 9d29fdd..b7e56294 100644 --- a/chrome/browser/webshare/win/scoped_share_operation_fake_components.h +++ b/chrome/browser/webshare/win/scoped_share_operation_fake_components.h @@ -34,6 +34,7 @@ void SetUp(); FakeDataTransferManagerInterop& fake_data_transfer_manager_interop(); + FakeDataWriterFactory& fake_data_writer_factory(); private: Microsoft::WRL::ComPtr<FakeDataWriterFactory> fake_data_writer_factory_; diff --git a/chrome/browser/webshare/win/share_operation_unittest.cc b/chrome/browser/webshare/win/share_operation_unittest.cc index a95acf3..1ba705f 100644 --- a/chrome/browser/webshare/win/share_operation_unittest.cc +++ b/chrome/browser/webshare/win/share_operation_unittest.cc @@ -19,6 +19,7 @@ #include "chrome/browser/webshare/win/fake_buffer.h" #include "chrome/browser/webshare/win/fake_data_transfer_manager.h" #include "chrome/browser/webshare/win/fake_data_transfer_manager_interop.h" +#include "chrome/browser/webshare/win/fake_data_writer_factory.h" #include "chrome/browser/webshare/win/scoped_share_operation_fake_components.h" #include "chrome/test/base/chrome_render_view_host_test_harness.h" #include "content/public/browser/browser_thread.h" @@ -191,6 +192,10 @@ return scoped_fake_components_.fake_data_transfer_manager_interop(); } + FakeDataWriterFactory& fake_data_writer_factory() { + return scoped_fake_components_.fake_data_writer_factory(); + } + private: raw_ptr<FakeDataTransferManager, DanglingUntriaged> fake_data_transfer_manager_ = nullptr; @@ -339,6 +344,7 @@ } TEST_F(ShareOperationUnitTest, SingleFileLargerThanSizeLimit) { + fake_data_writer_factory().SetCheckForUnflushedWriterDestroyed(false); bool post_data_requested_callback_invoked = false;
Loading diff…
Regression Test / PoC
shipped with the fix
diff --git a/chrome/browser/webshare/win/share_operation_unittest.cc b/chrome/browser/webshare/win/share_operation_unittest.cc
index a95acf3..1ba705f 100644
--- a/chrome/browser/webshare/win/share_operation_unittest.cc
+++ b/chrome/browser/webshare/win/share_operation_unittest.cc
@@ -19,6 +19,7 @@
#include "chrome/browser/webshare/win/fake_buffer.h"
#include "chrome/browser/webshare/win/fake_data_transfer_manager.h"
#include "chrome/browser/webshare/win/fake_data_transfer_manager_interop.h"
+#include "chrome/browser/webshare/win/fake_data_writer_factory.h"
#include "chrome/browser/webshare/win/scoped_share_operation_fake_components.h"
#include "chrome/test/base/chrome_render_view_host_test_harness.h"
#include "content/public/browser/browser_thread.h"
@@ -191,6 +192,10 @@
return scoped_fake_components_.fake_data_transfer_manager_interop();
}
+ FakeDataWriterFactory& fake_data_writer_factory() {
+ return scoped_fake_components_.fake_data_writer_factory();
+ }
+
private:
raw_ptr<FakeDataTransferManager, DanglingUntriaged>
fake_data_transfer_manager_ = nullptr;
@@ -339,6 +344,7 @@
}
TEST_F(ShareOperationUnitTest, SingleFileLargerThanSizeLimit) {
+ fake_data_writer_factory().SetCheckForUnflushedWriterDestroyed(false);
bool post_data_requested_callback_invoked = false;
ComPtr<IStorageFile> shared_file;
fake_data_transfer_manager()->SetPostDataRequestedCallback(
@@ -416,6 +422,8 @@
}
TEST_F(ShareOperationUnitTest, FilesTotallingLargerThanSizeLimit) {
+ fake_data_writer_factory().SetCheckForUnflushedWriterDestroyed(false);
+
bool post_data_requested_callback_invoked = false;
ComPtr<IStorageFile> shared_file_1;
ComPtr<IStorageFile> shared_file_2;
Loading diff…
Original Bug Report
reported by [email protected]
Potential UAF in browser process via off-thread destruction of OutputStreamWriteOperation
Project Fortify, an experimental security project, has identified the following potential security issue. If you’re a feature owner CC-ed on this bug, please do your best to review these reports. Please see https://chromium.googlesource.com/chromium/src/+/main/docs/security/ai-generated-security-bugs-faq.md for more information.
Overview: A potential Use-After-Free (UAF) vulnerability exists in the Windows-specific Web Share implementation. OutputStreamWriteOperation transitively owns resources that must only be destroyed on the IO thread. When its final reference is released on an arbitrary thread by Windows COM delegates, off-sequence destruction of non-atomic ref-counted blob items can lead to a race condition and subsequent memory corruption in the unsandboxed browser process.
Affected files:
chrome/browser/webshare/win/share_operation.ccstorage/browser/blob/blob_data_snapshot.hstorage/browser/blob/blob_data_item.h
Estimated timestamp from git blame: 2020-10-28
Root Cause Analysis
In chrome/browser/webshare/win/share_operation.cc, ShareOperation::PutShareContentInDataPackage creates an OutputStreamWriteOperation for each shared file and registers an IStreamedFileDataRequestedHandler COM delegate callback:
auto operation = base::MakeRefCounted<OutputStreamWriteOperation>(...);
...
auto raw_data_requested_callback =
Callback<IStreamedFileDataRequestedHandler>(
[operation](IOutputStream* stream) -> HRESULT {
operation->WriteStream(stream,
base::DoNothingWithBoundArgs(operation));
return S_OK;
});
The scoped_refptr<OutputStreamWriteOperation> is captured by the Windows Runtime (WRL) Callback object, which is passed to Windows via IStorageFileStatics::CreateStreamedFileAsync. Once the registration loop terminates, Windows retains the sole strong reference to this COM delegate.
When Windows eventually releases the delegate (e.g., when the target application releases the streamed StorageFile), the destruction callback runs on an arbitrary Windows thread-pool or COM thread. As a result, the ~OutputStreamWriteOperation destructor executes off-sequence on a non-IO thread.
Thread-Safety Violation on Blob Resources
During its execution, OutputStreamWriteOperation populates the following members on the IO thread:
writer_delegate_ = std::make_unique<storage::FileWriterDelegate>(
std::make_unique<DataWriterFileStreamWriter>(...),
storage::FlushPolicy::FLUSH_ON_COMPLETION);
writer_delegate_->Start(
blob_handle_->CreateReader(),
base::BindRepeating(&OutputStreamWriteOperation::OnFileWritten,
weak_factory_.GetWeakPtr()));
This sets up a ownership and destruction chain:
OutputStreamWriteOperationowns aFileWriterDelegate(storage/browser/file_system/file_writer_delegate.h).FileWriterDelegateowns aBlobReader(storage/browser/blob/blob_reader.h).BlobReaderowns aBlobDataSnapshot(storage/browser/blob/blob_reader.h).BlobDataSnapshotholds a list ofscoped_refptr<BlobDataItem>objects.
BlobDataSnapshot explicitly documents that it must be deleted on the IO thread. BlobDataItem uses non-atomic (non-thread-safe) reference counting:
class BlobDataItem : public base::RefCounted<BlobDataItem> { ... };
When ~OutputStreamWriteOperation executes on a non-IO thread, it triggers the synchronous destruction of FileWriterDelegate, BlobReader, and BlobDataSnapshot, performing non-atomic Release() decrements on BlobDataItem objects from an arbitrary background thread.
Potential Steps to Trigger
Because this is based on static code analysis, the following are potential steps that an attacker operating from a compromised renderer might use to trigger the race condition:
- The renderer initiates a Web Share operation containing a file associated with a specific Blob.
- Once the share target begins streaming the file data, the browser instantiates the IO-thread resources.
- The renderer concurrently and rapidly drives reference-count mutations on the same underlying Blob (e.g., via rapid
URL.createObjectURL/URL.revokeObjectURLcalls) to performAddRef()andRelease()operations on the IO thread. - When the file transfer finishes or is aborted, Windows releases the COM delegate off-thread, running
~BlobDataSnapshotand decrementing the non-atomic refcount ofBlobDataItemconcurrently. - The non-atomic race causes a lost reference increment/decrement, leading to a premature delete of a
BlobDataItemwhile a reference is still held in the registry. - A subsequent access to this item on the IO thread triggers a Use-After-Free (UAF) in the unsandboxed browser process.
Suggested Fix
To resolve this issue, ensure that OutputStreamWriteOperation and its thread-affine members are safely destroyed on the correct sequence. Because OutputStreamWriteOperation inherits from RefCountedThreadSafe, a custom deleter can be specified to force destruction on the IO thread:
class OutputStreamWriteOperation
: public base::RefCountedThreadSafe<OutputStreamWriteOperation,
content::BrowserThread::DeleteOnIOThread> {
// ...
};
Alternatively, specify a custom task runner when releasing sequence-affine members (like writer_delegate_ and blob_handle_) or post a task to delete them on the IO thread.
Evaluated with Chrome root at commit: a2bea94528f4bd6cc57739c43fa3bb890b8367d3
Results so far have been promising, but there can be wrong deductions. Feel free to adjust as follows:
- If you are familiar with the severity guidelines, you may adjust the severity.
- If this is a false positive, and there’s no work to be done, please close as WAI.
- If there is work to do here but not a vulnerability, please change the issue type to Task/Bug/FR.
Data from false positives will be used to improve accuracy over time. And please feel free to reach out to me directly if you have concerns or feedback on the project.
References
On This Page