CVE-2026-19141
Overview
Changed Functions
| Function | Change | Notes |
|---|---|---|
forui/base/resource/resource_bundle.cc |
modified | |
ifui/base/resource/resource_bundle.cc |
modified | |
max_scale_factor_ui/base/resource/resource_bundle.cc |
modified | |
TEST_Fui/base/resource/resource_bundle_unittest.cc |
modified | |
forui/base/resource/resource_bundle_unittest.cc |
modified |
Files Changed
ui/base/resource/resource_bundle.ccui/base/resource/resource_bundle.hui/base/resource/resource_bundle_unittest.cc
Patch
From 0a8eabbcb698270bf142d74f4616f7228c0f6bf8 Mon Sep 17 00:00:00 2001 From: Eriko Kurimoto <[email protected]> Date: Mon, 27 Jul 2026 21:14:29 -0700 Subject: [PATCH] Guard ResourceBundle::resource_handles_ with a lock ResourceBundle::GetRawDataResourceForScale() and HasDataResource() may run on a worker thread (e.g. via WebUIDataSourceImpl serving WebUI subresources) while AddResourceHandle() appends to resource_handles_ on the main thread when a feature module data pack is loaded after startup on Android. Iterating the vector while it reallocates is undefined. Add resource_handles_lock_ mirroring the existing locale_resources_data_lock_ and acquire it in AddResourceHandle() and the readers that may run off the main sequence. Bug: 513602949 Change-Id: I3ea13fed9a5e975c9a04627c9121bc2d8b3733aa Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8137184 Auto-Submit: Eriko Kurimoto <[email protected]> Reviewed-by: Dana Fried <[email protected]> Commit-Queue: Eriko Kurimoto <[email protected]> Cr-Commit-Position: refs/heads/main@{#1669194} --- diff --git a/ui/base/resource/resource_bundle.cc b/ui/base/resource/resource_bundle.cc index 2543039..fbdb939 100644 --- a/ui/base/resource/resource_bundle.cc +++ b/ui/base/resource/resource_bundle.cc @@ -730,6 +730,7 @@ if (delegate_ && delegate_->HasDataResource(resource_id)) { return true; } + base::AutoLock lock_scope(*resource_handles_lock_); for (const auto& resource_handle : resource_handles_) { if (resource_handle->HasResource(static_cast<uint16_t>(resource_id))) { return true; @@ -793,6 +794,8 @@ } } + base::AutoLock lock_scope(*resource_handles_lock_); + if (scale_factor != ui::k100Percent) { for (const auto& resource_handle : resource_handles_) { if (resource_handle->GetResourceScaleFactor() == scale_factor) { @@ -1014,6 +1017,7 @@ ResourceBundle::ResourceBundle(Delegate* delegate) : delegate_(delegate), locale_resources_data_lock_(new base::Lock), + resource_handles_lock_(new base::Lock), max_scale_factor_(k100Percent) { mangle_localized_strings_ = base::CommandLine::ForCurrentProcess()->HasSwitch( switches::kMangleLocalizedStrings); @@ -1105,6 +1109,7 @@ void ResourceBundle::AddResourceHandle( std::unique_ptr<ResourceHandle> resource_handle) { + base::AutoLock lock_scope(*resource_handles_lock_); #if DCHECK_IS_ON() resource_handle->CheckForDuplicateResources(resource_handles_); #endif @@ -1138,7 +1143,13 @@ } gfx::ImageSkia ResourceBundle::CreateImageSkia(int resource_id) { - DCHECK(!resource_handles_.empty()) << "Missing call to SetResourcesDataDLL?"; +#if DCHECK_IS_ON() + { + base::AutoLock lock_scope(*resource_handles_lock_); + DCHECK(!resource_handles_.empty()) + << "Missing call to SetResourcesDataDLL?"; + } +#endif std::optional<LottieData> data = GetLottieData(resource_id); if (data) { @@ -1197,6 +1208,7 @@ SkBitmap* bitmap, bool* fell_back_to_1x) const { DCHECK(fell_back_to_1x); + base::AutoLock lock_scope(*resource_handles_lock_); for (const auto& pack : resource_handles_) { if (pack->GetResourceScaleFactor() == ui::kScaleFactorNone && LoadBitmap(*pack, resource_id, bitmap, fell_back_to_1x)) { diff --git a/ui/base/resource/resource_bundle.h b/ui/base/resource/resource_bundle.h index 2ca1317..874d77b 100644 --- a/ui/base/resource/resource_bundle.h +++ b/ui/base/resource/resource_bundle.h @@ -573,6 +573,9 @@ // Protects |locale_resources_data_|. std::unique_ptr<base::Lock> locale_resources_data_lock_; + // Protects |resource_handles_|. + std::unique_ptr<base::Lock> resource_handles_lock_; + // Handles for data sources. std::vector<std::unique_ptr<ResourceHandle>> locale_resources_data_; std::vector<std::unique_ptr<ResourceHandle>> resource_handles_; diff --git a/ui/base/resource/resource_bundle_unittest.cc b/ui/base/resource/resource_bundle_unittest.cc index 9539471..2d5d3a24 100644 --- a/ui/base/resource/resource_bundle_unittest.cc +++ b/ui/base/resource/resource_bundle_unittest.cc @@ -8,6 +8,8 @@ #include <stdint.h> #include <algorithm> +#include <array> +#include <atomic> #include <map> #include <memory> #include <string> @@ -26,6 +28,8 @@ #include "base/numerics/byte_conversions.h" #include "base/strings/string_view_util.h" #include "base/strings/utf_string_conversions.h" +#include "base/test/bind.h" +#include "base/threading/thread.h" #include "build/build_config.h" #include "skia/buildflags.h" #include "testing/gmock/include/gmock/gmock.h" @@ -606,6 +610,45 @@ resource_bundle->GetRawDataResourceForScale(6, k200Percent)); } +// Data resources may be looked up on a worker thread while a data pack is +// being added on the main thread. Verify that this is supported. +TEST_F(ResourceBundleImageTest, GetRawDataResourceWhileAddingDataPack) { + base::FilePath empty_path = dir_path().Append(FILE_PATH_LITERAL("empty.pak")); + constexpr std::array<uint8_t, 15> kEmptyPakData = { + 0x04, 0x00, 0x00, 0x00, // header(version + 0x00, 0x00, 0x00, 0x00, // no. entries + 0x01, // encoding) + 0x00, 0x00, 0x0f, 0x00, 0x00, 0x00 // extra entry for the size of last + }; + ASSERT_TRUE(base::WriteFile(empty_path, kEmptyPakData)); + + ResourceBundle* resource_bundle = CreateResourceBundleWithEmptyLocalePak(); + resource_bundle->AddDataPackFromPath(empty_path, kScaleFactorNone); + + constexpr int kIterations = 256; + constexpr int kMissingResourceId = 42; + + std::atomic<bool> done = false; + base::Thread reader_thread("ResourceReader"); + ASSERT_TRUE(reader_thread.Start()); + reader_thread.task_runner()->PostTask( + FROM_HERE, base::BindLambdaForTesting([&]() { + while (!done.load()) { + EXPECT_FALSE(resource_bundle->HasDataResource(kMissingResourceId)); + EXPECT_TRUE( + resource_bundle->GetRawDataResource(kMissingResourceId).empty()); + } + })); + + for (int i = 0; i < kIterations; ++i) { + resource_bundle->AddDataPackFromPath(empty_path, kScaleFactorNone); + } + done.store(true); + reader_thread.Stop(); + + EXPECT_FALSE(resource_bundle->HasDataResource(kMissingResourceId)); +} + // Test requesting image reps at various scale factors from the image returned // via ResourceBundle::GetImageNamed(). TEST_F(ResourceBundleImageTest, GetImageNamed) {
Regression Test / PoC
diff --git a/ui/base/resource/resource_bundle_unittest.cc b/ui/base/resource/resource_bundle_unittest.cc
index 9539471..2d5d3a24 100644
--- a/ui/base/resource/resource_bundle_unittest.cc
+++ b/ui/base/resource/resource_bundle_unittest.cc
@@ -8,6 +8,8 @@
#include <stdint.h>
#include <algorithm>
+#include <array>
+#include <atomic>
#include <map>
#include <memory>
#include <string>
@@ -26,6 +28,8 @@
#include "base/numerics/byte_conversions.h"
#include "base/strings/string_view_util.h"
#include "base/strings/utf_string_conversions.h"
+#include "base/test/bind.h"
+#include "base/threading/thread.h"
#include "build/build_config.h"
#include "skia/buildflags.h"
#include "testing/gmock/include/gmock/gmock.h"
@@ -606,6 +610,45 @@
resource_bundle->GetRawDataResourceForScale(6, k200Percent));
}
+// Data resources may be looked up on a worker thread while a data pack is
+// being added on the main thread. Verify that this is supported.
+TEST_F(ResourceBundleImageTest, GetRawDataResourceWhileAddingDataPack) {
+ base::FilePath empty_path = dir_path().Append(FILE_PATH_LITERAL("empty.pak"));
+ constexpr std::array<uint8_t, 15> kEmptyPakData = {
+ 0x04, 0x00, 0x00, 0x00, // header(version
+ 0x00, 0x00, 0x00, 0x00, // no. entries
+ 0x01, // encoding)
+ 0x00, 0x00, 0x0f, 0x00, 0x00, 0x00 // extra entry for the size of last
+ };
+ ASSERT_TRUE(base::WriteFile(empty_path, kEmptyPakData));
+
+ ResourceBundle* resource_bundle = CreateResourceBundleWithEmptyLocalePak();
+ resource_bundle->AddDataPackFromPath(empty_path, kScaleFactorNone);
+
+ constexpr int kIterations = 256;
+ constexpr int kMissingResourceId = 42;
+
+ std::atomic<bool> done = false;
+ base::Thread reader_thread("ResourceReader");
+ ASSERT_TRUE(reader_thread.Start());
+ reader_thread.task_runner()->PostTask(
+ FROM_HERE, base::BindLambdaForTesting([&]() {
+ while (!done.load()) {
+ EXPECT_FALSE(resource_bundle->HasDataResource(kMissingResourceId));
+ EXPECT_TRUE(
+ resource_bundle->GetRawDataResource(kMissingResourceId).empty());
+ }
+ }));
+
+ for (int i = 0; i < kIterations; ++i) {
+ resource_bundle->AddDataPackFromPath(empty_path, kScaleFactorNone);
+ }
+ done.store(true);
+ reader_thread.Stop();
+
+ EXPECT_FALSE(resource_bundle->HasDataResource(kMissingResourceId));
+}
+
// Test requesting image reps at various scale factors from the image returned
// via ResourceBundle::GetImageNamed().
TEST_F(ResourceBundleImageTest, GetImageNamed) {
Original Bug Report
Potential Use-After-Free in ResourceBundle via Unsynchronized Access to resource_handles_
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 race condition in ui::ResourceBundle allows for a potential Use-After-Free (UAF) in the browser process, particularly on Android. Unsynchronized modification of the resource_handles_ vector during Dynamic Feature Module loading can cause memory reallocation while other threads are iterating the vector. This can result in virtual function calls being made on freed memory, potentially leading to a sandbox escape.
Affected files:
ui/base/resource/resource_bundle.ccui/base/resource/resource_bundle.hui/base/resource/resource_handle.h
Estimated timestamp from git blame: 2019-10-07
Summary
There is a potential race condition in ui::ResourceBundle due to unsynchronized access to the resource_handles_ vector. While ResourceBundle utilizes a lock for its locale_resources_data_ member, no such protection exists for resource_handles_. On Android, Dynamic Feature Modules (DFMs) such as dev_ui, ar, or vr load their resources at runtime, triggering an unsynchronized push_back on the UI thread. If this modification occurs while other threads (such as the IO thread or ThreadPool workers) are iterating over the same vector to resolve resources, a Use-After-Free (UAF) can occur if the vector reallocates its backing store.
Technical Details
In ui/base/resource/resource_bundle.h, resource_handles_ is defined as a std::vector<std::unique_ptr<ResourceHandle>>.
1. The Writer Path:
When an Android DFM is loaded (e.g., via ModuleInstaller), the native resources are registered through ResourceBundle::AddResourceHandle. This method performs a push_back on the vector without any thread synchronization:
// ui/base/resource/resource_bundle.cc
void ResourceBundle::AddResourceHandle(
std::unique_ptr<ResourceHandle> resource_handle) {
// ...
resource_handles_.push_back(std::move(resource_handle)); // No locking
}
2. The Reader Path:
Several frequently used methods iterate over this vector, such as GetRawDataResourceForScale and HasDataResource. These methods are called from various threads, including the IO thread (e.g., when generating filesystem:// directory listings via net::GetDirectoryListingHeader) and ThreadPool workers (e.g., when WebUI data sources fetch resources):
// ui/base/resource/resource_bundle.cc
for (const auto& resource_handle : resource_handles_) {
if (resource_handle->GetResourceScaleFactor() == scale_factor) { // Virtual call
// ...
3. The Vulnerability:
If the UI thread’s push_back triggers a vector reallocation, the old backing store is freed. An active iterator or reference on a reader thread then becomes stale. Because ResourceHandle is a polymorphic interface, subsequent virtual calls (like GetResourceScaleFactor) on the freed memory could allow an attacker to hijack control flow within the privileged browser process.
Suggested Attacker Steps
An attacker could potentially trigger this vulnerability from a compromised renderer process:
- Initiate a background task that triggers frequent resource lookups on the IO thread, such as repeated requests for
filesystem://URIs. - Simultaneously trigger the loading of an Android Dynamic Feature Module. This can be achieved by requesting a WebXR AR session or navigating the browser to a host served by the
dev_uimodule (e.g.,chrome://gpu). - If the timing is correct, the vector reallocation on the UI thread will collide with the iteration on the IO thread, resulting in a UAF.
Note: These steps are theoretical as we do not currently have a functional proof-of-concept.
Impact
A successful exploit would result in arbitrary code execution within the browser process, which is unsandboxed. This represents a full sandbox escape from a compromised renderer.
Recommended Fix
Access to resource_handles_ should be synchronized using a lock, similar to the existing implementation for locale_resources_data_. Alternatively, consider using a thread-safe container or ensuring all modifications and reads occur on the same sequence, though the latter may have significant performance implications given the current usage of ResourceBundle.
Evaluated with Chrome root at commit: 1a8d40fc44df2088d5945c0bf53584038aa1614a
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.