GMC: Fix UAF in MaybeRemoveDefaultDevice

This change eliminates a possible reference invalidation. Previously, a
pointer to an element in a vector was being dereferenced after erase
was called.

Bug: 1126351
Change-Id: Icfca65af49a1ddc716915dc16d5e4bb6ea45b149
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2401509Reviewed-by: Tommy Steimel <steimel@chromium.org>
Commit-Queue: Noah Rose Ledesma <noahrose@google.com>
Cr-Commit-Position: refs/heads/master@{#805458}

chrome/browser/ui/global_media_controls/media_notification_device_provider_impl.cc

@@ -44,10 +44,11 @@ void MaybeRemoveDefaultDevice(media::AudioDeviceDescriptions& descriptions) {
             default_device_name_prefix.size());
 
     // Find all the devices that have the name of the real default device.
-    std::vector<media::AudioDeviceDescription*> devices_with_real_default_name;
-    for (auto& description : descriptions) {
-      if (description.device_name == real_default_device_name) {
-        devices_with_real_default_name.push_back(&description);
+    std::vector<media::AudioDeviceDescriptions::iterator>
+        devices_with_real_default_name;
+    for (auto it = descriptions.begin(); it != descriptions.end(); ++it) {
+      if (it->device_name == real_default_device_name) {
+        devices_with_real_default_name.push_back(it);
       }
     }
 
@@ -56,9 +57,9 @@ void MaybeRemoveDefaultDevice(media::AudioDeviceDescriptions& descriptions) {
       // there is no ambiguity as to if this device is the real default device.
       // In this case, we should remove the "default" fallback device from the
       // list and mark the real device as "default".
-      descriptions.erase(default_device_it);
       devices_with_real_default_name.front()->unique_id =
           media::AudioDeviceDescription::kDefaultDeviceId;
+      descriptions.erase(default_device_it);
     }
   }
 }
@@ -127,7 +128,6 @@ void MediaNotificationDeviceProviderImpl::NotifySubscribers(
     media::AudioDeviceDescriptions descriptions) {
   is_querying_for_output_devices_ = false;
   audio_device_descriptions_ = std::move(descriptions);
-  MaybeRemoveDefaultDevice(audio_device_descriptions_);
   has_device_list_ = true;
   output_device_callback_list_.Notify(audio_device_descriptions_);
 }

chrome/browser/ui/global_media_controls/media_notification_device_provider_impl_unittest.cc

@@ -154,3 +154,26 @@ TEST(MediaNotificationDeviceProviderTest, NoDefaultDevice) {
   auto result = DescriptionsFromProvider(std::move(descriptions));
   EXPECT_TRUE(DescriptionsAreEqual(result, original_descriptions));
 }
+
+TEST(MediaNotificationDeviceProviderTest,
+     MaybeRemoveDefaultDeviceMultipleTimes) {
+  media::AudioDeviceDescriptions descriptions;
+  descriptions.emplace_back("Speaker", "1", "");
+  descriptions.emplace_back("Headphones", "2", "");
+  descriptions.emplace_back("Monitor", "3", "");
+  descriptions.emplace_back(
+      media::AudioDeviceDescription::GetDefaultDeviceName() + " - Speaker",
+      media::AudioDeviceDescription::kDefaultDeviceId, "");
+
+  media::AudioDeviceDescriptions expected_descriptions;
+  expected_descriptions.emplace_back(
+      "Speaker", media::AudioDeviceDescription::kDefaultDeviceId, "");
+  expected_descriptions.emplace_back("Headphones", "2", "");
+  expected_descriptions.emplace_back("Monitor", "3", "");
+
+  auto result = DescriptionsFromProvider(descriptions);
+  EXPECT_TRUE(DescriptionsAreEqual(result, expected_descriptions));
+  // Subsequent calls should not modify the devices list any further.
+  result = DescriptionsFromProvider(std::move(descriptions));
+  EXPECT_TRUE(DescriptionsAreEqual(result, expected_descriptions));
+}