[Backport] Security bug 382135228

Cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/6096837:
Replace raw pointers with scoped_refptr

VCM used VideoCaptureController raw pointers in a number of places,
including as a field in VCM::CaptureDeviceStartRequest.

This CL replaces the field and some other usages with a scoped_refptr
to prevent dangling pointers.

(cherry picked from commit 3524ce528548d1d743a6aa6e339ecb5a186c22bc)

Bug: 382135228
Change-Id: I1bd5f95bdf57631227034beb8bb076f258606378
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6088073
Commit-Queue: Guido Urdaneta <[email protected]>
Reviewed-by: Dale Curtis <[email protected]>
Cr-Original-Commit-Position: refs/heads/main@{#1396301}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6096837
Auto-Submit: Dale Curtis <[email protected]>
Commit-Queue: Dale Curtis <[email protected]>
Commit-Queue: Rubber Stamper <[email protected]>
Bot-Commit: Rubber Stamper <[email protected]>
Cr-Commit-Position: refs/branch-heads/6723@{#2725}
Cr-Branched-From: 985f2961df230630f9cbd75bd6fe463009855a11-refs/heads/main@{#1356013}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/615321
Reviewed-by: Anu Aliyas <[email protected]>

Author: Guido Urdaneta

chromium/content/browser/renderer_host/media/video_capture_manager.cc

@@ -15,6 +15,7 @@
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/memory/raw_ptr.h"
+#include "base/memory/scoped_refptr.h"
 #include "base/metrics/histogram_functions.h"
 #include "base/observer_list.h"
 #include "base/ranges/algorithm.h"
@@ -62,12 +63,14 @@ namespace content {
 class VideoCaptureManager::CaptureDeviceStartRequest {
  public:
   CaptureDeviceStartRequest(
-      VideoCaptureController* controller,
+      scoped_refptr<VideoCaptureController> controller,
       const media::VideoCaptureSessionId& session_id,
       const media::VideoCaptureParams& params,
       mojo::PendingRemote<video_effects::mojom::VideoEffectsProcessor>
           video_effects_processor);
-  VideoCaptureController* controller() const { return controller_; }
+  scoped_refptr<VideoCaptureController> controller() const {
+    return controller_;
+  }
   const base::UnguessableToken& session_id() const { return session_id_; }
   media::VideoCaptureParams params() const { return params_; }
 
@@ -77,20 +80,20 @@ class VideoCaptureManager::CaptureDeviceStartRequest {
   }
 
  private:
-  const raw_ptr<VideoCaptureController> controller_;
+  const scoped_refptr<VideoCaptureController> controller_;
   const base::UnguessableToken session_id_;
   const media::VideoCaptureParams params_;
   mojo::PendingRemote<video_effects::mojom::VideoEffectsProcessor>
       video_effects_processor_;
 };
 
 VideoCaptureManager::CaptureDeviceStartRequest::CaptureDeviceStartRequest(
-    VideoCaptureController* controller,
+    scoped_refptr<VideoCaptureController> controller,
     const media::VideoCaptureSessionId& session_id,
     const media::VideoCaptureParams& params,
     mojo::PendingRemote<video_effects::mojom::VideoEffectsProcessor>
         video_effects_processor)
-    : controller_(controller),
+    : controller_(std::move(controller)),
       session_id_(session_id),
       params_(params),
       video_effects_processor_(std::move(video_effects_processor)) {}
@@ -265,14 +268,15 @@ void VideoCaptureManager::ApplySubCaptureTarget(
 
 void VideoCaptureManager::QueueStartDevice(
     const media::VideoCaptureSessionId& session_id,
-    VideoCaptureController* controller,
+    scoped_refptr<VideoCaptureController> controller,
     const media::VideoCaptureParams& params,
     mojo::PendingRemote<video_effects::mojom::VideoEffectsProcessor>
         video_effects_processor) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(lock_time_.is_null());
-  device_start_request_queue_.push_back(CaptureDeviceStartRequest(
-      controller, session_id, params, std::move(video_effects_processor)));
+  device_start_request_queue_.push_back(
+      CaptureDeviceStartRequest(std::move(controller), session_id, params,
+                                std::move(video_effects_processor)));
   if (device_start_request_queue_.size() == 1)
     ProcessDeviceStartRequestQueue();
 }
@@ -318,7 +322,8 @@ void VideoCaptureManager::ProcessDeviceStartRequestQueue() {
   if (request == device_start_request_queue_.end())
     return;
 
-  VideoCaptureController* const controller = request->controller();
+  scoped_refptr<VideoCaptureController> const controller =
+      request->controller();
 
   EmitLogMessage("VideoCaptureManager::ProcessDeviceStartRequestQueue", 3);
   // The unit test VideoCaptureManagerTest.OpenNotExisting requires us to fail
@@ -336,7 +341,7 @@ void VideoCaptureManager::ProcessDeviceStartRequestQueue() {
         GetDeviceInfoById(controller->device_id());
     if (!device_info) {
       OnDeviceLaunchFailed(
-          controller,
+          controller.get(),
           media::VideoCaptureError::
               kVideoCaptureManagerProcessDeviceStartQueueDeviceInfoNotFound);
       return;
@@ -357,7 +362,7 @@ void VideoCaptureManager::ProcessDeviceStartRequestQueue() {
       base::BindOnce([](scoped_refptr<VideoCaptureManager>,
                         scoped_refptr<VideoCaptureController>) {},
                      scoped_refptr<VideoCaptureManager>(this),
-                     GetControllerSharedRef(controller)),
+                     std::move(controller)),
       request->TakeVideoEffectsProcessor());
 }
 
@@ -459,7 +464,7 @@ void VideoCaptureManager::ConnectClient(
     EmitLogMessage(string_stream.str(), 1);
   }
 
-  VideoCaptureController* controller =
+  scoped_refptr<VideoCaptureController> controller =
       GetOrCreateController(session_id, params);
   if (!controller) {
     std::move(done_cb).Run(nullptr);
@@ -949,7 +954,8 @@ media::VideoCaptureDeviceInfo* VideoCaptureManager::GetDeviceInfoById(
   return nullptr;
 }
 
-VideoCaptureController* VideoCaptureManager::GetOrCreateController(
+scoped_refptr<VideoCaptureController>
+VideoCaptureManager::GetOrCreateController(
     const media::VideoCaptureSessionId& capture_session_id,
     const media::VideoCaptureParams& params) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
@@ -971,10 +977,12 @@ VideoCaptureController* VideoCaptureManager::GetOrCreateController(
     return existing_device;
   }
 
-  VideoCaptureController* new_controller = new VideoCaptureController(
-      device_info.id, device_info.type, params,
-      video_capture_provider_->CreateDeviceLauncher(), emit_log_message_cb_);
-  controllers_.emplace_back(new_controller);
+  scoped_refptr<VideoCaptureController> new_controller =
+      base::MakeRefCounted<VideoCaptureController>(
+          device_info.id, device_info.type, params,
+          video_capture_provider_->CreateDeviceLauncher(),
+          emit_log_message_cb_);
+  controllers_.push_back(new_controller);
   return new_controller;
 }
 

chromium/content/browser/renderer_host/media/video_capture_manager.h

@@ -308,7 +308,7 @@ class CONTENT_EXPORT VideoCaptureManager
   // Finds a VideoCaptureController for the indicated |capture_session_id|,
   // creating a fresh one if necessary. Returns nullptr if said
   // |capture_session_id| is invalid.
-  VideoCaptureController* GetOrCreateController(
+  scoped_refptr<VideoCaptureController> GetOrCreateController(
       const media::VideoCaptureSessionId& capture_session_id,
       const media::VideoCaptureParams& params);
 
@@ -320,7 +320,7 @@ class CONTENT_EXPORT VideoCaptureManager
   // another request pending start.
   void QueueStartDevice(
       const media::VideoCaptureSessionId& session_id,
-      VideoCaptureController* controller,
+      scoped_refptr<VideoCaptureController> controller,
       const media::VideoCaptureParams& params,
       mojo::PendingRemote<video_effects::mojom::VideoEffectsProcessor>
           video_effects_processor);