Fix UAF in TtsPlatformImpl if a BrowserContext is deleted.

Bug: 1081350
Change-Id: I2b1824abefbd7fc3e8ce1c0cb433896161bab4e5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2211123Reviewed-by: David Tseng <dtseng@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#771222}

chrome/browser/speech/extension_api/tts_extension_apitest.cc

@@ -30,16 +30,17 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+using ::testing::_;
 using ::testing::AnyNumber;
 using ::testing::DoAll;
-using ::testing::Invoke;
 using ::testing::InSequence;
+using ::testing::Invoke;
 using ::testing::InvokeWithoutArgs;
+using ::testing::NiceMock;
 using ::testing::Return;
 using ::testing::SaveArg;
 using ::testing::SetArgPointee;
 using ::testing::StrictMock;
-using ::testing::_;
 
 namespace {
 int g_saved_utterance_id;
@@ -284,7 +285,13 @@ class TtsApiTest : public ExtensionApiTest {
     return false;
   }
 
+  void IgnoreAnyAdditionalCallsToMockPlatformImpl() {
+    content::TtsController::GetInstance()->SetTtsPlatform(
+        &nice_mock_platform_impl_);
+  }
+
   StrictMock<MockTtsPlatformImpl> mock_platform_impl_;
+  NiceMock<MockTtsPlatformImpl> nice_mock_platform_impl_;
 };
 
 IN_PROC_BROWSER_TEST_F(TtsApiTest, PlatformSpeakOptionalArgs) {
@@ -311,6 +318,7 @@ IN_PROC_BROWSER_TEST_F(TtsApiTest, PlatformSpeakOptionalArgs) {
   EXPECT_CALL(mock_platform_impl_, DoSpeak(_, "Echo", _, _, _))
       .WillOnce(Return());
   ASSERT_TRUE(RunExtensionTest("tts/optional_args")) << message_;
+  IgnoreAnyAdditionalCallsToMockPlatformImpl();
 }
 
 IN_PROC_BROWSER_TEST_F(TtsApiTest, PlatformSpeakFinishesImmediately) {

content/browser/browser_context.cc

@@ -43,6 +43,7 @@
 #include "content/browser/media/browser_feature_provider.h"
 #include "content/browser/permissions/permission_controller_impl.h"
 #include "content/browser/push_messaging/push_messaging_router.h"
+#include "content/browser/speech/tts_controller_impl.h"
 #include "content/browser/storage_partition_impl_map.h"
 #include "content/common/child_process_host_impl.h"
 #include "content/public/browser/blob_handle.h"
@@ -484,6 +485,8 @@ BrowserContext::~BrowserContext() {
   if (GetUserData(kDownloadManagerKeyName))
     GetDownloadManager(this)->Shutdown();
 
+  TtsControllerImpl::GetInstance()->OnBrowserContextDestroyed(this);
+
   TRACE_EVENT_NESTABLE_ASYNC_END1(
       "shutdown", "BrowserContext::NotifyWillBeDestroyed() called.", this,
       "browser_context", this);

content/browser/speech/tts_controller_impl.cc

@@ -98,14 +98,14 @@ void TtsControllerImpl::SpeakOrEnqueue(
   // If we're paused and we get an utterance that can't be queued,
   // flush the queue but stay in the paused state.
   if (paused_ && !utterance->GetCanEnqueue()) {
-    utterance_queue_.emplace(std::move(utterance));
+    utterance_deque_.emplace_back(std::move(utterance));
     Stop();
     paused_ = true;
     return;
   }
 
   if (paused_ || (IsSpeaking() && utterance->GetCanEnqueue())) {
-    utterance_queue_.emplace(std::move(utterance));
+    utterance_deque_.emplace_back(std::move(utterance));
   } else {
     Stop();
     SpeakNow(std::move(utterance));
@@ -113,11 +113,17 @@ void TtsControllerImpl::SpeakOrEnqueue(
 }
 
 void TtsControllerImpl::Stop() {
-  Stop(GURL());
+  StopInternal(GURL(), true);
 }
 
 void TtsControllerImpl::Stop(const GURL& source_url) {
-  base::RecordAction(base::UserMetricsAction("TextToSpeech.Stop"));
+  StopInternal(source_url, true);
+}
+
+void TtsControllerImpl::StopInternal(const GURL& source_url,
+                                     bool record_user_action) {
+  if (record_user_action)
+    base::RecordAction(base::UserMetricsAction("TextToSpeech.Stop"));
 
   paused_ = false;
 
@@ -272,13 +278,13 @@ void TtsControllerImpl::RemoveVoicesChangedDelegate(
 void TtsControllerImpl::RemoveUtteranceEventDelegate(
     UtteranceEventDelegate* delegate) {
   // First clear any pending utterances with this delegate.
-  base::queue<std::unique_ptr<TtsUtterance>> old_queue;
-  utterance_queue_.swap(old_queue);
-  while (!old_queue.empty()) {
-    std::unique_ptr<TtsUtterance> utterance = std::move(old_queue.front());
-    old_queue.pop();
+  std::deque<std::unique_ptr<TtsUtterance>> old_deque;
+  utterance_deque_.swap(old_deque);
+  while (!old_deque.empty()) {
+    std::unique_ptr<TtsUtterance> utterance = std::move(old_deque.front());
+    old_deque.pop_front();
     if (utterance->GetEventDelegate() != delegate)
-      utterance_queue_.emplace(std::move(utterance));
+      utterance_deque_.emplace_back(std::move(utterance));
   }
 
   if (current_utterance_ &&
@@ -313,12 +319,38 @@ TtsEngineDelegate* TtsControllerImpl::GetTtsEngineDelegate() {
   return GetTtsControllerDelegate()->GetTtsEngineDelegate();
 }
 
+void TtsControllerImpl::OnBrowserContextDestroyed(
+    BrowserContext* browser_context) {
+  bool did_clear_utterances = false;
+
+  // First clear the BrowserContext from any utterances.
+  for (std::unique_ptr<TtsUtterance>& utterance : utterance_deque_) {
+    if (utterance->GetBrowserContext() == browser_context) {
+      utterance->ClearBrowserContext();
+      did_clear_utterances = true;
+    }
+  }
+
+  if (current_utterance_ &&
+      current_utterance_->GetBrowserContext() == browser_context) {
+    current_utterance_->ClearBrowserContext();
+    did_clear_utterances = true;
+  }
+
+  // If we cleared the BrowserContext from any utterances, stop speech
+  // just to be safe. Don't record user actions because that crashes in
+  // tests because last_active_profile_ isn't cleared.
+  if (did_clear_utterances) {
+    StopInternal(GURL(), false);
+  }
+}
+
 void TtsControllerImpl::SetTtsPlatform(TtsPlatform* tts_platform) {
   tts_platform_ = tts_platform;
 }
 
 int TtsControllerImpl::QueueSize() {
-  return static_cast<int>(utterance_queue_.size());
+  return static_cast<int>(utterance_deque_.size());
 }
 
 TtsPlatform* TtsControllerImpl::GetTtsPlatform() {
@@ -417,7 +449,7 @@ void TtsControllerImpl::OnSpeakFinished(int utterance_id, bool success) {
   // the browser has built-in TTS that isn't loaded yet.
   if (GetTtsPlatform()->LoadBuiltInTtsEngine(
           current_utterance_->GetBrowserContext())) {
-    utterance_queue_.emplace(std::move(current_utterance_));
+    utterance_deque_.emplace_back(std::move(current_utterance_));
     return;
   }
 
@@ -427,10 +459,10 @@ void TtsControllerImpl::OnSpeakFinished(int utterance_id, bool success) {
 }
 
 void TtsControllerImpl::ClearUtteranceQueue(bool send_events) {
-  while (!utterance_queue_.empty()) {
+  while (!utterance_deque_.empty()) {
     std::unique_ptr<TtsUtterance> utterance =
-        std::move(utterance_queue_.front());
-    utterance_queue_.pop();
+        std::move(utterance_deque_.front());
+    utterance_deque_.pop_front();
     if (send_events) {
       utterance->OnTtsEvent(TTS_EVENT_CANCELLED, kInvalidCharIndex,
                             kInvalidLength, std::string());
@@ -455,10 +487,10 @@ void TtsControllerImpl::SpeakNextUtterance() {
 
   // Start speaking the next utterance in the queue.  Keep trying in case
   // one fails but there are still more in the queue to try.
-  while (!utterance_queue_.empty() && !current_utterance_) {
+  while (!utterance_deque_.empty() && !current_utterance_) {
     std::unique_ptr<TtsUtterance> utterance =
-        std::move(utterance_queue_.front());
-    utterance_queue_.pop();
+        std::move(utterance_deque_.front());
+    utterance_deque_.pop_front();
     SpeakNow(std::move(utterance));
   }
 }

content/browser/speech/tts_controller_impl.h

@@ -5,12 +5,12 @@
 #ifndef CONTENT_BROWSER_SPEECH_TTS_CONTROLLER_IMPL_H_
 #define CONTENT_BROWSER_SPEECH_TTS_CONTROLLER_IMPL_H_
 
+#include <deque>
 #include <memory>
 #include <set>
 #include <string>
 #include <vector>
 
-#include "base/containers/queue.h"
 #include "base/gtest_prod_util.h"
 #include "base/json/json_reader.h"
 #include "base/macros.h"
@@ -59,6 +59,10 @@ class CONTENT_EXPORT TtsControllerImpl : public TtsController {
   void SetTtsEngineDelegate(TtsEngineDelegate* delegate) override;
   TtsEngineDelegate* GetTtsEngineDelegate() override;
 
+  // Called directly by ~BrowserContext, because a raw BrowserContext pointer
+  // is stored in an Utterance.
+  void OnBrowserContextDestroyed(BrowserContext* browser_context);
+
   // Testing methods
   void SetTtsPlatform(TtsPlatform* tts_platform) override;
   int QueueSize() override;
@@ -77,6 +81,7 @@ class CONTENT_EXPORT TtsControllerImpl : public TtsController {
   FRIEND_TEST_ALL_PREFIXES(TtsControllerTest, TestGetMatchingVoice);
   FRIEND_TEST_ALL_PREFIXES(TtsControllerTest,
                            TestTtsControllerUtteranceDefaults);
+  FRIEND_TEST_ALL_PREFIXES(TtsControllerTest, TestBrowserContextRemoved);
 
   friend struct base::DefaultSingletonTraits<TtsControllerImpl>;
 
@@ -87,6 +92,10 @@ class CONTENT_EXPORT TtsControllerImpl : public TtsController {
   // |utterance| or delete it if there's an error. Returns true on success.
   void SpeakNow(std::unique_ptr<TtsUtterance> utterance);
 
+  // Implementation of Stop(), with an optional flag to indicate whether
+  // user actions should be recorded.
+  void StopInternal(const GURL& source_url, bool record_user_action);
+
   // Clear the utterance queue. If send_events is true, will send
   // TTS_EVENT_CANCELLED events on each one.
   void ClearUtteranceQueue(bool send_events);
@@ -131,7 +140,7 @@ class CONTENT_EXPORT TtsControllerImpl : public TtsController {
   TtsPlatform* tts_platform_;
 
   // A queue of utterances to speak after the current one finishes.
-  base::queue<std::unique_ptr<TtsUtterance>> utterance_queue_;
+  std::deque<std::unique_ptr<TtsUtterance>> utterance_deque_;
 
   DISALLOW_COPY_AND_ASSIGN(TtsControllerImpl);
 };

content/browser/speech/tts_controller_unittest.cc

@@ -9,6 +9,8 @@
 #include "content/browser/speech/tts_controller_impl.h"
 #include "content/public/browser/tts_controller_delegate.h"
 #include "content/public/browser/tts_platform.h"
+#include "content/public/test/browser_task_environment.h"
+#include "content/public/test/test_browser_context.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/blink/public/mojom/speech/speech_synthesis.mojom.h"
 
@@ -50,9 +52,15 @@ class MockTtsControllerDelegate : public TtsControllerDelegate {
   MockTtsControllerDelegate() {}
   ~MockTtsControllerDelegate() override {}
 
+  BrowserContext* GetLastBrowserContext() {
+    BrowserContext* result = last_browser_context_;
+    last_browser_context_ = nullptr;
+    return result;
+  }
+
   int GetMatchingVoice(content::TtsUtterance* utterance,
                        std::vector<content::VoiceData>& voices) override {
-    // Below 0 implies a "native" voice.
+    last_browser_context_ = utterance->GetBrowserContext();
     return -1;
   }
 
@@ -66,6 +74,9 @@ class MockTtsControllerDelegate : public TtsControllerDelegate {
   content::TtsEngineDelegate* GetTtsEngineDelegate() override {
     return nullptr;
   }
+
+ private:
+  BrowserContext* last_browser_context_ = nullptr;
 };
 
 // Subclass of TtsController with a public ctor and dtor.
@@ -101,6 +112,45 @@ TEST_F(TtsControllerTest, TestTtsControllerShutdown) {
   delete delegate;
 }
 
+TEST_F(TtsControllerTest, TestBrowserContextRemoved) {
+  // Create a controller, mock other stuff, and create a test
+  // browser context.
+  TtsControllerImpl* controller = TtsControllerImpl::GetInstance();
+  MockTtsPlatformImpl platform_impl;
+  MockTtsControllerDelegate delegate;
+  controller->delegate_ = &delegate;
+  controller->SetTtsPlatform(&platform_impl);
+  content::BrowserTaskEnvironment task_environment;
+  auto browser_context = std::make_unique<TestBrowserContext>();
+
+  // Speak an utterances associated with this test browser context.
+  std::unique_ptr<TtsUtterance> utterance1 =
+      TtsUtterance::Create(browser_context.get());
+  utterance1->SetCanEnqueue(true);
+  utterance1->SetSrcId(1);
+  controller->SpeakOrEnqueue(std::move(utterance1));
+
+  // Assert that the delegate was called and it got our browser context.
+  ASSERT_EQ(browser_context.get(), delegate.GetLastBrowserContext());
+
+  // Now queue up a second utterance to be spoken, also associated with
+  // this browser context.
+  std::unique_ptr<TtsUtterance> utterance2 =
+      TtsUtterance::Create(browser_context.get());
+  utterance2->SetCanEnqueue(true);
+  utterance2->SetSrcId(2);
+  controller->SpeakOrEnqueue(std::move(utterance2));
+
+  // Destroy the browser context before the utterance is spoken.
+  browser_context.reset();
+
+  // Now speak the next utterance, and ensure that we don't get the
+  // destroyed browser context.
+  controller->FinishCurrentUtterance();
+  controller->SpeakNextUtterance();
+  ASSERT_EQ(nullptr, delegate.GetLastBrowserContext());
+}
+
 #if !defined(OS_CHROMEOS)
 TEST_F(TtsControllerTest, TestTtsControllerUtteranceDefaults) {
   std::unique_ptr<TtsControllerForTesting> controller =

content/browser/speech/tts_utterance_impl.cc

@@ -182,6 +182,10 @@ BrowserContext* TtsUtteranceImpl::GetBrowserContext() {
   return browser_context_;
 }
 
+void TtsUtteranceImpl::ClearBrowserContext() {
+  browser_context_ = nullptr;
+}
+
 int TtsUtteranceImpl::GetId() {
   return id_;
 }

content/browser/speech/tts_utterance_impl.h

@@ -68,6 +68,8 @@ class CONTENT_EXPORT TtsUtteranceImpl : public TtsUtterance {
   UtteranceEventDelegate* GetEventDelegate() override;
 
   BrowserContext* GetBrowserContext() override;
+  void ClearBrowserContext() override;
+
   int GetId() override;
   bool IsFinished() override;
 
@@ -102,7 +104,7 @@ class CONTENT_EXPORT TtsUtteranceImpl : public TtsUtterance {
   GURL src_url_;
 
   // The delegate to be called when an utterance event is fired.
-  UtteranceEventDelegate* event_delegate_;
+  UtteranceEventDelegate* event_delegate_ = nullptr;
 
   // The parsed options.
   std::string voice_name_;

content/public/browser/tts_utterance.h

@@ -118,6 +118,7 @@ class CONTENT_EXPORT TtsUtterance {
 
   // Getters and setters for internal state.
   virtual BrowserContext* GetBrowserContext() = 0;
+  virtual void ClearBrowserContext() = 0;
   virtual int GetId() = 0;
   virtual bool IsFinished() = 0;
 };