Skip to content

Commit 954d60b

Browse files
Guido Urdanetamibrunin
Guido Urdaneta
authored andcommitted
[Backport] Security bug 382135228
Manual cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/6088073: [VideoCaptureManager] 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. 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-Commit-Position: refs/heads/main@{#1396301} Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/615714 Reviewed-by: Anu Aliyas <[email protected]>
1 parent 8d6028e commit 954d60b

File tree

2 files changed

+25
-18
lines changed

2 files changed

+25
-18
lines changed

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

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include "base/location.h"
1717
#include "base/logging.h"
1818
#include "base/memory/raw_ptr.h"
19+
#include "base/memory/scoped_refptr.h"
1920
#include "base/metrics/histogram_functions.h"
2021
#include "base/observer_list.h"
2122
#include "base/ranges/algorithm.h"
@@ -57,24 +58,26 @@ namespace content {
5758
// Class used for queuing request for starting a device.
5859
class VideoCaptureManager::CaptureDeviceStartRequest {
5960
public:
60-
CaptureDeviceStartRequest(VideoCaptureController* controller,
61+
CaptureDeviceStartRequest(scoped_refptr<VideoCaptureController> controller,
6162
const media::VideoCaptureSessionId& session_id,
6263
const media::VideoCaptureParams& params);
63-
VideoCaptureController* controller() const { return controller_; }
64+
scoped_refptr<VideoCaptureController> controller() const {
65+
return controller_;
66+
}
6467
const base::UnguessableToken& session_id() const { return session_id_; }
6568
media::VideoCaptureParams params() const { return params_; }
6669

6770
private:
68-
const raw_ptr<VideoCaptureController> controller_;
71+
const scoped_refptr<VideoCaptureController> controller_;
6972
const base::UnguessableToken session_id_;
7073
const media::VideoCaptureParams params_;
7174
};
7275

7376
VideoCaptureManager::CaptureDeviceStartRequest::CaptureDeviceStartRequest(
74-
VideoCaptureController* controller,
77+
scoped_refptr<VideoCaptureController> controller,
7578
const media::VideoCaptureSessionId& session_id,
7679
const media::VideoCaptureParams& params)
77-
: controller_(controller), session_id_(session_id), params_(params) {}
80+
: controller_(std::move(controller)), session_id_(session_id), params_(params) {}
7881

7982
VideoCaptureManager::VideoCaptureManager(
8083
std::unique_ptr<VideoCaptureProvider> video_capture_provider,
@@ -236,12 +239,12 @@ void VideoCaptureManager::Crop(
236239

237240
void VideoCaptureManager::QueueStartDevice(
238241
const media::VideoCaptureSessionId& session_id,
239-
VideoCaptureController* controller,
242+
scoped_refptr<VideoCaptureController> controller,
240243
const media::VideoCaptureParams& params) {
241244
DCHECK_CURRENTLY_ON(BrowserThread::IO);
242245
DCHECK(lock_time_.is_null());
243246
device_start_request_queue_.push_back(
244-
CaptureDeviceStartRequest(controller, session_id, params));
247+
CaptureDeviceStartRequest(std::move(controller), session_id, params));
245248
if (device_start_request_queue_.size() == 1)
246249
ProcessDeviceStartRequestQueue();
247250
}
@@ -287,7 +290,8 @@ void VideoCaptureManager::ProcessDeviceStartRequestQueue() {
287290
if (request == device_start_request_queue_.end())
288291
return;
289292

290-
VideoCaptureController* const controller = request->controller();
293+
scoped_refptr<VideoCaptureController> const controller =
294+
request->controller();
291295

292296
EmitLogMessage("VideoCaptureManager::ProcessDeviceStartRequestQueue", 3);
293297
// The unit test VideoCaptureManagerTest.OpenNotExisting requires us to fail
@@ -305,7 +309,7 @@ void VideoCaptureManager::ProcessDeviceStartRequestQueue() {
305309
GetDeviceInfoById(controller->device_id());
306310
if (!device_info) {
307311
OnDeviceLaunchFailed(
308-
controller,
312+
controller.get(),
309313
media::VideoCaptureError::
310314
kVideoCaptureManagerProcessDeviceStartQueueDeviceInfoNotFound);
311315
return;
@@ -326,7 +330,7 @@ void VideoCaptureManager::ProcessDeviceStartRequestQueue() {
326330
base::BindOnce([](scoped_refptr<VideoCaptureManager>,
327331
scoped_refptr<VideoCaptureController>) {},
328332
scoped_refptr<VideoCaptureManager>(this),
329-
GetControllerSharedRef(controller)));
333+
std::move(controller)));
330334
}
331335

332336
void VideoCaptureManager::OnDeviceLaunched(VideoCaptureController* controller) {
@@ -408,7 +412,7 @@ void VideoCaptureManager::ConnectClient(
408412
EmitLogMessage(string_stream.str(), 1);
409413
}
410414

411-
VideoCaptureController* controller =
415+
scoped_refptr<VideoCaptureController> controller =
412416
GetOrCreateController(session_id, params);
413417
if (!controller) {
414418
std::move(done_cb).Run(nullptr);
@@ -871,7 +875,8 @@ media::VideoCaptureDeviceInfo* VideoCaptureManager::GetDeviceInfoById(
871875
return nullptr;
872876
}
873877

874-
VideoCaptureController* VideoCaptureManager::GetOrCreateController(
878+
scoped_refptr<VideoCaptureController>
879+
VideoCaptureManager::GetOrCreateController(
875880
const media::VideoCaptureSessionId& capture_session_id,
876881
const media::VideoCaptureParams& params) {
877882
DCHECK_CURRENTLY_ON(BrowserThread::IO);
@@ -893,10 +898,12 @@ VideoCaptureController* VideoCaptureManager::GetOrCreateController(
893898
return existing_device;
894899
}
895900

896-
VideoCaptureController* new_controller = new VideoCaptureController(
897-
device_info.id, device_info.type, params,
898-
video_capture_provider_->CreateDeviceLauncher(), emit_log_message_cb_);
899-
controllers_.emplace_back(new_controller);
901+
scoped_refptr<VideoCaptureController> new_controller =
902+
base::MakeRefCounted<VideoCaptureController>(
903+
device_info.id, device_info.type, params,
904+
video_capture_provider_->CreateDeviceLauncher(),
905+
emit_log_message_cb_);
906+
controllers_.push_back(new_controller);
900907
return new_controller;
901908
}
902909

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,7 @@ class CONTENT_EXPORT VideoCaptureManager
281281
// Finds a VideoCaptureController for the indicated |capture_session_id|,
282282
// creating a fresh one if necessary. Returns nullptr if said
283283
// |capture_session_id| is invalid.
284-
VideoCaptureController* GetOrCreateController(
284+
scoped_refptr<VideoCaptureController> GetOrCreateController(
285285
const media::VideoCaptureSessionId& capture_session_id,
286286
const media::VideoCaptureParams& params);
287287

@@ -293,7 +293,7 @@ class CONTENT_EXPORT VideoCaptureManager
293293
// request to start the device on the device thread unless there is
294294
// another request pending start.
295295
void QueueStartDevice(const media::VideoCaptureSessionId& session_id,
296-
VideoCaptureController* controller,
296+
scoped_refptr<VideoCaptureController> controller,
297297
const media::VideoCaptureParams& params);
298298
void DoStopDevice(VideoCaptureController* controller);
299299
void ProcessDeviceStartRequestQueue();

0 commit comments

Comments
 (0)