Chrome · ReadAloud › Reading mode
CVE-2026-78934
Race in ReadAloud (Reading mode)
Overview
High
Severity
8.3
CVSS
No
Exploited ITW
Fixed
Fix Status
Background
- Reading mode / Read Aloud
- Chrome’s Reading mode (
read_anything) shows a distilled copy of the page. Its Read Aloud mode speaks that text and can highlight it phrase by phrase, a path the tests label “PhrasesEnabled”. - `DependencyParserModel`
- a TFLite-backed parser in
chrome/renderer/accessibility/phrase_segmentation/that computes dependency heads for a list of word tokens so phrase boundaries can be found. The model file is loaded throughUpdateWithFile(), and per the commit message its TFLite dependencies are not thread-safe. - Process-wide singleton
GetDependencyParserModel_()returns onebase::NoDestructorinstance that every Reading mode controller in the same renderer process shares.- `base::SequenceBound<T>`
- a wrapper that owns a
Ton one specific sequenced task runner and allows access only throughAsyncCall(). Every method call on the object is therefore serialized on that sequence.
Root Cause Analysis
Before the patch, the process-wide DependencyParserModel was a plain static base::NoDestructor<DependencyParserModel> that was reached from more than one thread with no synchronization. Phrase segmentation in read_aloud_app_model.cc posted the free function GetDependencyHeads with base::ThreadPool::PostTaskAndReplyWithResult. That uses an unsequenced pool, so several inference tasks could run TFLite on the shared model at the same time. Meanwhile ReadAnythingAppController::UpdateDependencyParserModel() called UpdateWithFile(), and the controller’s setup code called IsAvailable(), directly on the calling (renderer main) thread. A model update could therefore replace or reinitialize the TFLite state while a worker thread was running inference on it. Because the TFLite dependencies are not thread-safe, these overlapping calls are a data race on the interpreter and model state.
The patch does not show what memory-safety consequence followed, such as a use after free of the old model buffers; that is (inferred).
The fix wraps the singleton in base::SequenceBound<DependencyParserModel> bound to one ThreadPool::CreateSequencedTaskRunner, and turns every access (GetDependencyHeads, UpdateWithFile, IsAvailable) into an AsyncCall, so all operations run one at a time on a single sequence. It also changes DependencyParserModel::GetDependencyHeads from DCHECK(IsAvailable()) to a runtime if (!IsAvailable()) return {};. The availability check and the inference now run atomically on the same sequence, instead of being checked by a caller beforehand.
Key insight
A non-thread-safe TFLite model was shared across the whole process and called both from unsequenced thread-pool tasks and from the main thread with no serialization. Binding the model to a single sequence with
base::SequenceBound makes every load, availability check and inference run in order on one sequence.Attack Path
- Open Reading mode with phrase highlighting
The user opens Reading mode and uses Read Aloud on a page, with phrase-based highlighting active. Whether that requires a non-default feature is not shown in the diff. This creates a
ReadAnythingAppControllerwhoseReadAloudAppModeluses the process-wideDependencyParserModel. - Concurrent inference tasks
As text is segmented, the phrase-segmentation code (the enclosing function is not named in the hunk) posts a
GetDependencyHeadstask to the unsequenced thread pool for each batch ofphrase_tokens, so several TFLite inferences can overlap. The page text decides how much segmentation work happens (inferred). - Model update on another thread
At the same time, the browser delivers or refreshes the model file and
UpdateDependencyParserModel()callsUpdateWithFile()on the main thread. Several Reading mode instances in one renderer process can also share the singleton (inferred). - Racy access to TFLite state
Inference and model reload touch the same non-thread-safe TFLite objects at once. This can corrupt memory or use freed model state in the renderer that hosts Reading mode
(inferred).
Impact Assessment
The bug is a race condition, which the advisory calls “Race condition in ReadAloud”, in the renderer process hosting the Reading mode / Read Aloud UI. It can corrupt TFLite parser state or cause a use after free
(inferred). It needs user interaction: opening Reading mode and using Read Aloud with phrase segmentation. The attacker cannot control timing well, because the race depends on inference running alongside a model load or another inference. The vendor advisory rates this high severity and does not report in-the-wild exploitation; NVD scores it CVSS 8.3. Any code execution in that renderer is (inferred).Changed Functions
| Function | Change | Notes |
|---|---|---|
ReadAloudAppModelchrome/renderer/accessibility/read_anything/read_aloud_app_model.cc |
modified | |
GetDependencyParserModelchrome/renderer/accessibility/read_anything/read_aloud_app_model.cc |
modified | |
ifchrome/renderer/accessibility/read_anything/read_anything_app_controller.cc |
modified | |
GetDependencyParserModelForTestingchrome/renderer/accessibility/read_anything/read_anything_app_controller.cc |
modified | |
MockAXTreeDistillerchrome/renderer/accessibility/read_anything/read_anything_app_controller_browsertest.cc |
modified |
Files Changed
chrome/renderer/accessibility/read_anything/read_anything_app_controller_browsertest.ccchrome/renderer/accessibility/phrase_segmentation/dependency_parser_model_unittest.ccchrome/renderer/accessibility/phrase_segmentation/dependency_parser_model.ccchrome/renderer/accessibility/phrase_segmentation/dependency_parser_model.hchrome/renderer/accessibility/read_anything/read_anything_app_controller.hchrome/renderer/accessibility/read_anything/read_anything_app_controller.ccchrome/renderer/accessibility/read_anything/read_aloud_app_model_browsertest.ccchrome/renderer/accessibility/read_anything/read_aloud_app_model.ccchrome/renderer/accessibility/read_anything/read_aloud_app_model.h
Audit Directions
- Shared ML model singletonsSearch
chrome/renderer/andcomponents/forstatic base::NoDestructor<...Model>or other process-wide TFLite/ML wrappers that are reached fromThreadPool::PostTask*without a sequenced runner, especially where the same object also loads or reloads its model from another thread. - Unsequenced `PostTaskAndReplyWithResult` on shared stateAny
base::ThreadPool::PostTaskAndReplyWithResultwhose task function reaches a global or singleton must be checked. Without aSequencedTaskRunnerorSequenceBound, separate posts can run in parallel. - Check-then-use across threadsLook for a caller checking an
IsAvailable()/IsReady()predicate on one thread and then calling the guarded method on another, with the guard as aDCHECK. Move the check into the method so it runs on the same sequence, as this patch did.
Patch
From 73f0ba56be18d7a345ac87e8c64bf098a46bf995 Mon Sep 17 00:00:00 2001 From: Lauren Winston <[email protected]> Date: Wed, 29 Jul 2026 20:45:00 -0700 Subject: [PATCH] [Reading mode] Sequence-bind the phrase segmentation This CL refactors the phrase segmentation architecture to ensure all operations on the process-wide DependencyParserModel are properly sequenced, preventing unsafe concurrent execution, since the TFLite dependencies are not thread-safe. Fixed:517550232 Phrase highlighting Change-Id: I03e5d3c3ecb218e5a529d0011a7e0d044a919b26 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8162836 Reviewed-by: Kristi Saney <[email protected]> Commit-Queue: Lauren Winston <[email protected]> Cr-Commit-Position: refs/heads/main@{#1670808} --- diff --git a/chrome/renderer/accessibility/phrase_segmentation/dependency_parser_model.cc b/chrome/renderer/accessibility/phrase_segmentation/dependency_parser_model.cc index fe0b85d..11c1d8a 100644 --- a/chrome/renderer/accessibility/phrase_segmentation/dependency_parser_model.cc +++ b/chrome/renderer/accessibility/phrase_segmentation/dependency_parser_model.cc @@ -113,8 +113,13 @@ } std::vector<size_t> DependencyParserModel::GetDependencyHeads( - base::span<const std::string> input) { - DCHECK(IsAvailable()); + const std::vector<std::string>& input) { + // IsAvailable() might return false if the model file is not completely + // downloaded yet, if the file was invalid, or if the underlying TFLite engine + // failed to initialize. + if (!IsAvailable()) { + return {}; + } base::ElapsedTimer timer; // Perform the following operations to identify the dependency heads for each diff --git a/chrome/renderer/accessibility/phrase_segmentation/dependency_parser_model.h b/chrome/renderer/accessibility/phrase_segmentation/dependency_parser_model.h index beb6256..d31a49b 100644 --- a/chrome/renderer/accessibility/phrase_segmentation/dependency_parser_model.h +++ b/chrome/renderer/accessibility/phrase_segmentation/dependency_parser_model.h @@ -60,7 +60,7 @@ // Runs the TFLite dependency parser model on a string. This will return // a vector of dependency head for each word in the string. - std::vector<size_t> GetDependencyHeads(base::span<const std::string> input); + std::vector<size_t> GetDependencyHeads(const std::vector<std::string>& input); private: // Returns the dependency head of each node in a dependency graph. The input diff --git a/chrome/renderer/accessibility/read_anything/read_aloud_app_model.cc b/chrome/renderer/accessibility/read_anything/read_aloud_app_model.cc index 03781c6..39f914f 100644 --- a/chrome/renderer/accessibility/read_anything/read_aloud_app_model.cc +++ b/chrome/renderer/accessibility/read_anything/read_aloud_app_model.cc @@ -28,18 +28,15 @@ namespace { // Returns the dependency parser model for this renderer process. -DependencyParserModel& GetDependencyParserModel_() { - static base::NoDestructor<DependencyParserModel> instance; +// All access to this model is asynchronous and safely executed on a background +// sequenced task runner. +base::SequenceBound<DependencyParserModel>& GetDependencyParserModel_() { + static base::NoDestructor<base::SequenceBound<DependencyParserModel>> + instance(base::ThreadPool::CreateSequencedTaskRunner( + {base::MayBlock(), base::TaskPriority::BEST_EFFORT})); return *instance; } -std::vector<size_t> GetDependencyHeads(base::span<const std::string> input) { - DependencyParserModel& dependency_parser_model = GetDependencyParserModel_(); - return dependency_parser_model.IsAvailable() - ? dependency_parser_model.GetDependencyHeads(input) - : std::vector<size_t>(); -} - } // namespace ReadAloudAppModel::ReadAloudAppModel() { @@ -156,7 +153,8 @@ } } -DependencyParserModel& ReadAloudAppModel::GetDependencyParserModel() { +base::SequenceBound<DependencyParserModel>& +ReadAloudAppModel::GetDependencyParserModel() { return GetDependencyParserModel_(); } @@ -210,11 +208,11 @@ static_cast<std::string (*)(std::u16string_view)>(&base::UTF16ToUTF8)); // Perform computation of dependency heads asynchronously. - base::ThreadPool::PostTaskAndReplyWithResult( - FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT}, - base::BindOnce(&GetDependencyHeads, phrase_tokens), - base::BindOnce(&ReadAloudAppModel::UpdatePhraseBoundaries, - weak_ptr_factory_.GetWeakPtr(), phrase_tokens)); + GetDependencyParserModel_() + .AsyncCall(&DependencyParserModel::GetDependencyHeads) + .WithArgs(phrase_tokens) + .Then(base::BindOnce(&ReadAloudAppModel::UpdatePhraseBoundaries, + weak_ptr_factory_.GetWeakPtr(), phrase_tokens)); } static const Strategy kPhraseStrategy = Strategy::kWords; diff --git a/chrome/renderer/accessibility/read_anything/read_aloud_app_model.h b/chrome/renderer/accessibility/read_anything/read_aloud_app_model.h index b99e1c9..21df015a 100644 --- a/chrome/renderer/accessibility/read_anything/read_aloud_app_model.h +++ b/chrome/renderer/accessibility/read_anything/read_aloud_app_model.h @@ -6,6 +6,7 @@ #define CHROME_RENDERER_ACCESSIBILITY_READ_ANYTHING_READ_ALOUD_APP_MODEL_H_ #include "base/metrics/single_sample_metrics.h" +#include "base/threading/sequence_bound.h" #include "base/values.h" #include "chrome/common/read_anything/read_anything.mojom.h" #include "chrome/renderer/accessibility/phrase_segmentation/dependency_parser_model.h" @@ -136,7 +137,7 @@ const std::set<ui::AXNodeID>* current_nodes); // Get the dependency parsing model for this renderer process. - DependencyParserModel& GetDependencyParserModel(); + base::SequenceBound<DependencyParserModel>& GetDependencyParserModel(); // Increments the processed_granularity_index_, updating ReadAloud's state of // the current granularity to refer to the next granularity. The current diff --git a/chrome/renderer/accessibility/read_anything/read_anything_app_controller.cc b/chrome/renderer/accessibility/read_anything/read_anything_app_controller.cc index ec9c3e0..be13140 100644 --- a/chrome/renderer/accessibility/read_anything/read_anything_app_controller.cc +++ b/chrome/renderer/accessibility/read_anything/read_anything_app_controller.cc @@ -2475,8 +2475,18 @@ render_frame()->GetBrowserInterfaceBroker().GetInterface( std::move(page_handler_factory_receiver)); - // Get the dependency parser model used by phrase-based highlighting. - if (read_aloud_model_.GetDependencyParserModel().IsAvailable()) { + // Asynchronously check the availability of the sequence-bound dependency + // parser model used by phrase-based highlighting. + read_aloud_model_.GetDependencyParserModel() + .AsyncCall(&DependencyParserModel::IsAvailable) + .Then(base::BindOnce(&ReadAnythingAppController:: + OnDependencyParserModelAvailabilityChecked, + weak_ptr_factory_.GetWeakPtr())); +} + +void ReadAnythingAppController::OnDependencyParserModelAvailabilityChecked( + bool is_available) { + if (is_available) { return; } @@ -3212,11 +3222,12 @@ void ReadAnythingAppController::UpdateDependencyParserModel( base::File model_file) { - read_aloud_model_.GetDependencyParserModel().UpdateWithFile( - std::move(model_file)); + read_aloud_model_.GetDependencyParserModel() + .AsyncCall(&DependencyParserModel::UpdateWithFile) + .WithArgs(std::move(model_file)); } -DependencyParserModel& +base::SequenceBound<DependencyParserModel>& ReadAnythingAppController::GetDependencyParserModelForTesting() { return read_aloud_model_.GetDependencyParserModel(); } diff --git a/chrome/renderer/accessibility/read_anything/read_anything_app_controller.h b/chrome/renderer/accessibility/read_anything/read_anything_app_controller.h index e5e192d1..103b4e3 100644 --- a/chrome/renderer/accessibility/read_anything/read_anything_app_controller.h +++ b/chrome/renderer/accessibility/read_anything/read_anything_app_controller.h @@ -16,6 +16,7 @@ #include "base/memory/raw_ptr.h" #include "base/memory/safe_ref.h" #include "base/scoped_observation.h" +#include "base/threading/sequence_bound.h" #include "base/values.h" #include "chrome/common/read_anything/read_anything.mojom.h" #include "chrome/renderer/accessibility/read_anything/read_aloud_app_model.h" @@ -403,7 +404,10 @@ // available. void UpdateDependencyParserModel(base::File model_file); - DependencyParserModel& GetDependencyParserModelForTesting(); + void OnDependencyParserModelAvailabilityChecked(bool is_available); + + base::SequenceBound<DependencyParserModel>& + GetDependencyParserModelForTesting(); // Called when distillation has completed. void OnAXTreeDistilled(const ui::AXTreeID& tree_id, diff --git a/chrome/renderer/accessibility/phrase_segmentation/dependency_parser_model_unittest.cc b/chrome/renderer/accessibility/phrase_segmentation/dependency_parser_model_unittest.cc index 551d1d3b..f9d0fe1 100644 --- a/chrome/renderer/accessibility/phrase_segmentation/dependency_parser_model_unittest.cc +++ b/chrome/renderer/accessibility/phrase_segmentation/dependency_parser_model_unittest.cc @@ -127,9 +127,9 @@ // # - cream(11) // # children: // # - or(10) - static const auto input = std::to_array<std::string>( - {"Ice", "cream", "is", "a", "frozen", "dessert", "typically", "made", - "from", "milk", "or", "cream"}); + const std::vector<std::string> input = { + "Ice", "cream", "is", "a", "frozen", "dessert", + "typically", "made", "from", "milk", "or", "cream"}; auto prediction = dependency_parser_model_->GetDependencyHeads(input); EXPECT_THAT(prediction, ::testing::ElementsAre(1, 5, 5, 5, 5, 5, 7, 5, 9, 7, 11, 9)); diff --git a/chrome/renderer/accessibility/read_anything/read_aloud_app_model_browsertest.cc b/chrome/renderer/accessibility/read_anything/read_aloud_app_model_browsertest.cc index d21fdb1..600a88cd 100644 --- a/chrome/renderer/accessibility/read_anything/read_aloud_app_model_browsertest.cc +++ b/chrome/renderer/accessibility/read_anything/read_aloud_app_model_browsertest.cc @@ -10,6 +10,7 @@ #include "base/memory/raw_ptr.h" #include "base/test/metrics/histogram_tester.h" #include "base/test/scoped_feature_list.h" +#include "base/test/test_future.h" #include "base/time/time.h" #include "chrome/renderer/accessibility/read_anything/read_aloud_traversal_utils.h" #include "chrome/test/base/chrome_render_view_test.h" @@ -430,10 +431,17 @@ TEST_F( ReadAnythingReadAloudAppModelV8SegmentationTest, GetHighlightForCurrentSegmentIndex_PhrasesEnabled_ValidModel_SentenceSpansMultipleNodes_ReturnsCorrectNodes) { - model().GetDependencyParserModel().UpdateWithFile(test::GetValidModelFile()); - DependencyParserModel& phrase_model = model().GetDependencyParserModel(); + model() + .GetDependencyParserModel() + .AsyncCall(&DependencyParserModel::UpdateWithFile) + .WithArgs(test::GetValidModelFile()); - EXPECT_TRUE(phrase_model.IsAvailable()); + base::test::TestFuture<bool> future; + model() + .GetDependencyParserModel() + .AsyncCall(&DependencyParserModel::IsAvailable) + .Then(future.GetCallback()); + EXPECT_TRUE(future.Get()); // Text indices: 0123456789012345678901234567890 std::u16string sentence1 = u"Never feel heavy or "; diff --git a/chrome/renderer/accessibility/read_anything/read_anything_app_controller_browsertest.cc b/chrome/renderer/accessibility/read_anything/read_anything_app_controller_browsertest.cc index 5d99de4f..688cfb8 100644 --- a/chrome/renderer/accessibility/read_anything/read_anything_app_controller_browsertest.cc +++ b/chrome/renderer/accessibility/read_anything/read_anything_app_controller_browsertest.cc @@ -18,6 +18,7 @@ #include "base/test/gtest_util.h" #include "base/test/metrics/histogram_tester.h" #include "base/test/scoped_feature_list.h" +#include "base/test/test_future.h" #include "build/build_config.h" #include "chrome/common/read_anything/read_anything.mojom-shared.h" #include "chrome/common/read_anything/read_anything_util.h" @@ -48,9 +49,7 @@ #include "v8/include/v8-context.h" #include "v8/include/v8-microtask-queue.h" -namespace { - -} // namespace +namespace {} // namespace class MockAXTreeDistiller : public AXTreeDistiller { public: @@ -1878,8 +1877,6 @@ EXPECT_EQ(true, controller().IsLeafNode(4)); } - - TEST_F(ReadAnythingAppControllerTest, DisplayNodeIdsContains_NoSelectionOrContentNodes) { OnAXTreeDistilled(tree_id_, {}); @@ -2331,8 +2328,6 @@ Mock::VerifyAndClearExpectations(distiller_); } - - TEST_F(ReadAnythingAppControllerTest, ScrollToTargetNode_DoesNotScrollIfNotGoogleDocs) { ui::AXNodeData root; @@ -2790,31 +2785,35 @@ TEST_F(ReadAnythingAppControllerTest, GetDependencyParserModel_UnavailableWithoutModelFile) { - DependencyParserModel& model = - controller().GetDependencyParserModelForTesting(); - EXPECT_FALSE(model.IsAvailable()); + auto& model = controller().GetDependencyParserModelForTesting(); + base::test::TestFuture<bool> future; + model.AsyncCall(&DependencyParserModel::IsAvailable) + .Then(future.GetCallback()); + EXPECT_FALSE(future.Get()); } TEST_F(ReadAnythingAppControllerTest, GetDependencyParserModel_AvailableWithValidModelFile) { controller().UpdateDependencyParserModel(test::GetValidModelFile()); - DependencyParserModel& model = - controller().GetDependencyParserModelForTesting(); + auto& model = controller().GetDependencyParserModelForTesting(); - EXPECT_TRUE(model.IsAvailable()); + base::test::TestFuture<bool> future; + model.AsyncCall(&DependencyParserModel::IsAvailable) + .Then(future.GetCallback()); + EXPECT_TRUE(future.Get()); } TEST_F(ReadAnythingAppControllerTest, GetDependencyParserModel_UnavailableWithInvalidModelFile) { controller().UpdateDependencyParserModel(test::GetInvalidModelFile()); - DependencyParserModel& model = - controller().GetDependencyParserModelForTesting(); + auto& model = controller().GetDependencyParserModelForTesting(); - EXPECT_FALSE(model.IsAvailable()); + base::test::TestFuture<bool> future; + model.AsyncCall(&DependencyParserModel::IsAvailable) + .Then(future.GetCallback()); + EXPECT_FALSE(future.Get()); } - - TEST_F(ReadAnythingAppControllerTest, OnStringAttributeChanged_NonImageNode_DoesNothing) { base::test::ScopedFeatureList feature_list; @@ -2957,8 +2956,6 @@ EXPECT_EQ(0, model().words_heard()); } - - TEST_F(ReadAnythingAppControllerTest, ProcessPendingUpdatesIfAllowed_ExitsIfNoTree) { // Destroy existing tree created in Setup(). @@ -6028,8 +6025,6 @@ RecordScreen2xDistillationStatus(/*just_hidden=*/false); } - - TEST_F(ReadAnythingAppControllerTest, Screen2xDistillationStatus_DoesNotRelogOnReopenWithoutRedistillation) { base::test::ScopedFeatureList scoped_feature_list;
Loading diff…
Regression Test / PoC
shipped with the fix
diff --git a/chrome/renderer/accessibility/phrase_segmentation/dependency_parser_model_unittest.cc b/chrome/renderer/accessibility/phrase_segmentation/dependency_parser_model_unittest.cc
index 551d1d3b..f9d0fe1 100644
--- a/chrome/renderer/accessibility/phrase_segmentation/dependency_parser_model_unittest.cc
+++ b/chrome/renderer/accessibility/phrase_segmentation/dependency_parser_model_unittest.cc
@@ -127,9 +127,9 @@
// # - cream(11)
// # children:
// # - or(10)
- static const auto input = std::to_array<std::string>(
- {"Ice", "cream", "is", "a", "frozen", "dessert", "typically", "made",
- "from", "milk", "or", "cream"});
+ const std::vector<std::string> input = {
+ "Ice", "cream", "is", "a", "frozen", "dessert",
+ "typically", "made", "from", "milk", "or", "cream"};
auto prediction = dependency_parser_model_->GetDependencyHeads(input);
EXPECT_THAT(prediction,
::testing::ElementsAre(1, 5, 5, 5, 5, 5, 7, 5, 9, 7, 11, 9));
diff --git a/chrome/renderer/accessibility/read_anything/read_aloud_app_model_browsertest.cc b/chrome/renderer/accessibility/read_anything/read_aloud_app_model_browsertest.cc
index d21fdb1..600a88cd 100644
--- a/chrome/renderer/accessibility/read_anything/read_aloud_app_model_browsertest.cc
+++ b/chrome/renderer/accessibility/read_anything/read_aloud_app_model_browsertest.cc
@@ -10,6 +10,7 @@
#include "base/memory/raw_ptr.h"
#include "base/test/metrics/histogram_tester.h"
#include "base/test/scoped_feature_list.h"
+#include "base/test/test_future.h"
#include "base/time/time.h"
#include "chrome/renderer/accessibility/read_anything/read_aloud_traversal_utils.h"
#include "chrome/test/base/chrome_render_view_test.h"
@@ -430,10 +431,17 @@
TEST_F(
ReadAnythingReadAloudAppModelV8SegmentationTest,
GetHighlightForCurrentSegmentIndex_PhrasesEnabled_ValidModel_SentenceSpansMultipleNodes_ReturnsCorrectNodes) {
- model().GetDependencyParserModel().UpdateWithFile(test::GetValidModelFile());
- DependencyParserModel& phrase_model = model().GetDependencyParserModel();
+ model()
+ .GetDependencyParserModel()
+ .AsyncCall(&DependencyParserModel::UpdateWithFile)
+ .WithArgs(test::GetValidModelFile());
- EXPECT_TRUE(phrase_model.IsAvailable());
+ base::test::TestFuture<bool> future;
+ model()
+ .GetDependencyParserModel()
+ .AsyncCall(&DependencyParserModel::IsAvailable)
+ .Then(future.GetCallback());
+ EXPECT_TRUE(future.Get());
// Text indices: 0123456789012345678901234567890
std::u16string sentence1 = u"Never feel heavy or ";
diff --git a/chrome/renderer/accessibility/read_anything/read_anything_app_controller_browsertest.cc b/chrome/renderer/accessibility/read_anything/read_anything_app_controller_browsertest.cc
index 5d99de4f..688cfb8 100644
--- a/chrome/renderer/accessibility/read_anything/read_anything_app_controller_browsertest.cc
+++ b/chrome/renderer/accessibility/read_anything/read_anything_app_controller_browsertest.cc
@@ -18,6 +18,7 @@
#include "base/test/gtest_util.h"
#include "base/test/metrics/histogram_tester.h"
#include "base/test/scoped_feature_list.h"
+#include "base/test/test_future.h"
#include "build/build_config.h"
#include "chrome/common/read_anything/read_anything.mojom-shared.h"
#include "chrome/common/read_anything/read_anything_util.h"
@@ -48,9 +49,7 @@
#include "v8/include/v8-context.h"
#include "v8/include/v8-microtask-queue.h"
-namespace {
-
-} // namespace
+namespace {} // namespace
class MockAXTreeDistiller : public AXTreeDistiller {
public:
@@ -1878,8 +1877,6 @@
EXPECT_EQ(true, controller().IsLeafNode(4));
}
-
-
TEST_F(ReadAnythingAppControllerTest,
DisplayNodeIdsContains_NoSelectionOrContentNodes) {
OnAXTreeDistilled(tree_id_, {});
@@ -2331,8 +2328,6 @@
Mock::VerifyAndClearExpectations(distiller_);
}
-
-
TEST_F(ReadAnythingAppControllerTest,
ScrollToTargetNode_DoesNotScrollIfNotGoogleDocs) {
ui::AXNodeData root;
@@ -2790,31 +2785,35 @@
TEST_F(ReadAnythingAppControllerTest,
GetDependencyParserModel_UnavailableWithoutModelFile) {
- DependencyParserModel& model =
- controller().GetDependencyParserModelForTesting();
- EXPECT_FALSE(model.IsAvailable());
+ auto& model = controller().GetDependencyParserModelForTesting();
+ base::test::TestFuture<bool> future;
+ model.AsyncCall(&DependencyParserModel::IsAvailable)
+ .Then(future.GetCallback());
+ EXPECT_FALSE(future.Get());
}
TEST_F(ReadAnythingAppControllerTest,
GetDependencyParserModel_AvailableWithValidModelFile) {
controller().UpdateDependencyParserModel(test::GetValidModelFile());
- DependencyParserModel& model =
- controller().GetDependencyParserModelForTesting();
+ auto& model = controller().GetDependencyParserModelForTesting();
- EXPECT_TRUE(model.IsAvailable());
+ base::test::TestFuture<bool> future;
+ model.AsyncCall(&DependencyParserModel::IsAvailable)
+ .Then(future.GetCallback());
+ EXPECT_TRUE(future.Get());
}
TEST_F(ReadAnythingAppControllerTest,
GetDependencyParserModel_UnavailableWithInvalidModelFile) {
controller().UpdateDependencyParserModel(test::GetInvalidModelFile());
- DependencyParserModel& model =
- controller().GetDependencyParserModelForTesting();
+ auto& model = controller().GetDependencyParserModelForTesting();
- EXPECT_FALSE(model.IsAvailable());
+ base::test::TestFuture<bool> future;
+ model.AsyncCall(&DependencyParserModel::IsAvailable)
+ .Then(future.GetCallback());
+ EXPECT_FALSE(future.Get());
}
-
-
TEST_F(ReadAnythingAppControllerTest,
OnStringAttributeChanged_NonImageNode_DoesNothing) {
base::test::ScopedFeatureList feature_list;
@@ -2957,8 +2956,6 @@
EXPECT_EQ(0, model().words_heard());
}
-
-
TEST_F(ReadAnythingAppControllerTest,
ProcessPendingUpdatesIfAllowed_ExitsIfNoTree) {
// Destroy existing tree created in Setup().
@@ -6028,8 +6025,6 @@
RecordScreen2xDistillationStatus(/*just_hidden=*/false);
}
-
-
TEST_F(ReadAnythingAppControllerTest,
Screen2xDistillationStatus_DoesNotRelogOnReopenWithoutRedistillation) {
base::test::ScopedFeatureList scoped_feature_list;
Loading diff…
Original Bug Report
reported by [email protected]
Potential Heap Use-After-Free via concurrent access to shared DependencyParserModel in ThreadPool
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 concurrency race condition exists in the Reading Mode phrase segmentation code. Multiple ReadAloudAppModel instances in the same renderer process can concurrently access the process-wide DependencyParserModel singleton via unsequenced ThreadPool tasks. Because the underlying TfLiteEngine and tflite::Interpreter are not thread-safe, concurrent operations can result in a heap Use-After-Free or out-of-bounds write.
Affected files:
chrome/renderer/accessibility/read_anything/read_aloud_app_model.ccchrome/renderer/accessibility/phrase_segmentation/dependency_parser_model.ccchrome/renderer/accessibility/read_anything/read_aloud_app_model.hchrome/renderer/accessibility/phrase_segmentation/dependency_parser_model.h
Estimated timestamp from git blame: 2024-12-18
Description
A potential concurrency race condition has been identified in the phrase segmentation logic of the Reading Mode feature.
In chrome/renderer/accessibility/read_anything/read_aloud_app_model.cc, the process-wide DependencyParserModel singleton is retrieved via a static base::NoDestructor instance in GetDependencyParserModel_():
DependencyParserModel& GetDependencyParserModel_() {
static base::NoDestructor<DependencyParserModel> instance;
return *instance;
}
When ReadAloudAppModel::CalculatePhrases is invoked, it schedules the inference task asynchronously using base::ThreadPool::PostTaskAndReplyWithResult on an unsequenced task runner:
base::ThreadPool::PostTaskAndReplyWithResult(
FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT},
base::BindOnce(&GetDependencyHeads, phrase_tokens),
base::BindOnce(&ReadAloudAppModel::UpdatePhraseBoundaries,
weak_ptr_factory_.GetWeakPtr(), phrase_tokens));
Although ReadAloudAppModel uses an instance-level boolean guard (is_calculating_phrases) to prevent concurrent re-entry within a single instance, it does not prevent concurrent calculations across different instances in the same process.
When multiple Reading Mode side panels or tabs are consolidated into a single renderer process (such as when the browser process limit is reached), different ReadAloudAppModel instances can post tasks concurrently. Because the ThreadPool runner is unsequenced, these tasks run concurrently on separate background worker threads.
Concurrency Races on tflite::Interpreter
Both background threads will concurrently execute GetDependencyHeads(), which directly accesses the shared DependencyParserModel singleton and its underlying tflite::Interpreter:
auto* interpreter = dependency_parser_model_->interpreter();
interpreter->ResizeInputTensor(0, {1, static_cast<int>(input.size())});
TfLiteTensor* input_tensor = interpreter->input_tensor(0);
...
input_buffer.WriteToTensor(input_tensor, /*new_shape=*/nullptr);
interpreter->AllocateTensors();
interpreter->Invoke();
As documented, tflite::Interpreter is not thread-safe. Concurrent execution of ResizeInputTensor, AllocateTensors, and Invoke on separate worker threads will cause data races, corrupt the internal allocator structures, and lead to a heap Use-After-Free or heap Out-of-Bounds write when the interpreter references freed/invalidated tensor arena blocks.
Main Thread vs. Worker Thread Race
An additional race exists when the browser process sends an updated model file to the renderer via Mojo. ReadAnythingAppController::UpdateDependencyParserModel is executed on the main thread, which calls DependencyParserModel::UpdateWithFile(). This method re-initializes the interpreter:
dependency_parser_model_ = std::move(tflite_engine);
If this re-initialization occurs on the main thread while a ThreadPool worker thread is concurrently running interpreter->Invoke(), the underlying TfLiteEngine object is destroyed from under the running thread, leading to a severe use-after-free.
Note: These are potential steps based on source code analysis; our automated tooling does not currently have the capability to run code or compile a live proof-of-concept.
Suggested Potential Reproduction Steps
- Enable the
ReadAnythingReadAloudPhraseHighlightingfeature flag. - Open multiple tabs to force renderer process consolidation so that subsequent Reading Mode instances share the same renderer process.
- Simultaneously trigger phrase calculations in multiple instances (e.g., starting Read Aloud on different documents).
- Under ASan/TSan, this is expected to yield a heap Use-After-Free or a data race warning within the TFLite interpreter.
Recommended Fix
To resolve this issue, access to the shared DependencyParserModel must be serialized.
- Avoid executing
GetDependencyHeadson an unsequenced ThreadPool runner. Instead, define a sequenced task runner specifically for the process-wide dependency parser model operations, ensuring all inference tasks run sequentially on the same sequence:
scoped_refptr<base::SequencedTaskRunner> GetModelTaskRunner() {
static base::NoDestructor<scoped_refptr<base::SequencedTaskRunner>> runner(
base::ThreadPool::CreateSequencedTaskRunner(
{base::MayBlock(), base::TaskPriority::BEST_EFFORT}));
return *runner;
}
- Ensure both
CalculatePhrasestask posting andUpdateWithFilemodel updates are synchronized or dispatched onto this same sequenced task runner.
Evaluated with Chrome root at commit: 5133b93d189b383c37805b1cf3a9d2dbfe8d7379
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