Chrome · Media Stream
CVE-2025-1921
Logic Error in Media Stream
Overview
Medium
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Changed Functions
| Function | Change | Notes |
|---|---|---|
switchthird_party/blink/renderer/modules/mediastream/media_devices.cc |
modified | |
ifthird_party/blink/renderer/modules/mediastream/media_devices.cc |
modified | |
MockMediaPermissionthird_party/blink/renderer/modules/mediastream/media_devices_test.cc |
modified | |
ifthird_party/blink/renderer/modules/mediastream/media_devices_test.cc |
modified | |
MediaDevicesTestthird_party/blink/renderer/modules/mediastream/media_devices_test.cc |
modified | |
platformthird_party/blink/renderer/modules/mediastream/media_devices_test.cc |
modified |
Files Changed
third_party/blink/renderer/modules/mediastream/BUILD.gnthird_party/blink/renderer/modules/mediastream/media_devices.ccthird_party/blink/renderer/modules/mediastream/media_devices.hthird_party/blink/renderer/modules/mediastream/media_devices_test.cc
Patch
From 5182f8756d86cb441d3445fcb628f961e74bd6ca Mon Sep 17 00:00:00 2001 From: Guido Urdaneta <[email protected]> Date: Wed, 08 Jan 2025 08:27:06 -0800 Subject: [PATCH] [MediaDevices] Check permission before firing devicechange event Bug: 387583503 Change-Id: Iffb85a335719c263555bb4a2630da90cb756189d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6152197 Reviewed-by: Palak Agarwal <[email protected]> Commit-Queue: Guido Urdaneta <[email protected]> Cr-Commit-Position: refs/heads/main@{#1403590} --- diff --git a/third_party/blink/renderer/modules/mediastream/BUILD.gn b/third_party/blink/renderer/modules/mediastream/BUILD.gn index 9cd83406..82c640a 100644 --- a/third_party/blink/renderer/modules/mediastream/BUILD.gn +++ b/third_party/blink/renderer/modules/mediastream/BUILD.gn @@ -148,6 +148,8 @@ testonly = true sources = [ + "media_permission_testing_platform.cc", + "media_permission_testing_platform.h", "mock_constraint_factory.cc", "mock_constraint_factory.h", "mock_encoded_video_frame.h", diff --git a/third_party/blink/renderer/modules/mediastream/media_devices.cc b/third_party/blink/renderer/modules/mediastream/media_devices.cc index 8121a0be..53563ef3 100644 --- a/third_party/blink/renderer/modules/mediastream/media_devices.cc +++ b/third_party/blink/renderer/modules/mediastream/media_devices.cc @@ -14,6 +14,7 @@ #include "base/strings/strcat.h" #include "base/uuid.h" #include "build/build_config.h" +#include "media/base/media_permission.h" #include "mojo/public/cpp/bindings/remote.h" #include "third_party/blink/public/common/features.h" #include "third_party/blink/public/common/mediastream/media_devices.h" @@ -24,7 +25,9 @@ #include "third_party/blink/public/mojom/mediastream/media_devices.mojom-blink.h" #include "third_party/blink/public/mojom/permissions_policy/permissions_policy_feature.mojom-blink.h" #include "third_party/blink/public/platform/browser_interface_broker_proxy.h" +#include "third_party/blink/public/platform/platform.h" #include "third_party/blink/public/platform/task_type.h" +#include "third_party/blink/public/web/web_local_frame.h" #include "third_party/blink/renderer/bindings/core/v8/dictionary.h" #include "third_party/blink/renderer/bindings/core/v8/script_promise.h" #include "third_party/blink/renderer/bindings/core/v8/script_promise_resolver.h" @@ -389,6 +392,19 @@ } #endif // !BUILDFLAG(IS_ANDROID) && !BUILDFLAG(IS_IOS) +media::MediaPermission::Type ToMediaPermissionType( + mojom::blink::MediaDeviceType media_device_type) { + switch (media_device_type) { + case mojom::blink::MediaDeviceType::kMediaAudioInput: + case mojom::blink::MediaDeviceType::kMediaAudioOutput: + return media::MediaPermission::Type::kAudioCapture; + case mojom::blink::MediaDeviceType::kMediaVideoInput: + return media::MediaPermission::Type::kVideoCapture; + case mojom::blink::MediaDeviceType::kNumMediaDeviceTypes: + NOTREACHED(); + } +} + } // namespace const char MediaDevices::kSupplementName[] = "MediaDevices"; @@ -1121,6 +1137,20 @@ current_device_infos_[static_cast<wtf_size_t>(type)] = device_infos; if (RuntimeEnabledFeatures::OnDeviceChangeEnabled()) { + if (media::MediaPermission* media_permission = + blink::Platform::Current()->GetWebRTCMediaPermission( + WebLocalFrame::FromFrameToken( + DomWindow()->GetLocalFrameToken()))) { + media_permission->HasPermission( + ToMediaPermissionType(type), + WTF::BindOnce(&MediaDevices::MaybeFireDeviceChangeEvent, + WrapWeakPersistent(this))); + } + } +} + +void MediaDevices::MaybeFireDeviceChangeEvent(bool has_permission) { + if (has_permission) { ScheduleDispatchEvent(Event::Create(event_type_names::kDevicechange)); } } diff --git a/third_party/blink/renderer/modules/mediastream/media_devices.h b/third_party/blink/renderer/modules/mediastream/media_devices.h index 813b4de..eb8583b 100644 --- a/third_party/blink/renderer/modules/mediastream/media_devices.h +++ b/third_party/blink/renderer/modules/mediastream/media_devices.h @@ -136,6 +136,8 @@ void OnDevicesChanged(mojom::blink::MediaDeviceType, const Vector<WebMediaDeviceInfo>&) override; + void MaybeFireDeviceChangeEvent(bool has_permission); + void SetDispatcherHostForTesting( mojo::PendingRemote<mojom::blink::MediaDevicesDispatcherHost>); diff --git a/third_party/blink/renderer/modules/mediastream/media_devices_test.cc b/third_party/blink/renderer/modules/mediastream/media_devices_test.cc index bcf42030..3ec93f4 100644 --- a/third_party/blink/renderer/modules/mediastream/media_devices_test.cc +++ b/third_party/blink/renderer/modules/mediastream/media_devices_test.cc @@ -9,6 +9,7 @@ #include "base/test/metrics/histogram_tester.h" #include "build/build_config.h" +#include "media/base/media_permission.h" #include "media/base/output_device_info.h" #include "media/base/video_types.h" #include "media/capture/mojom/video_capture_types.mojom.h" @@ -50,6 +51,7 @@ #include "third_party/blink/renderer/modules/mediastream/crop_target.h" #include "third_party/blink/renderer/modules/mediastream/input_device_info.h" #include "third_party/blink/renderer/modules/mediastream/media_device_info.h" +#include "third_party/blink/renderer/modules/mediastream/media_permission_testing_platform.h" #include "third_party/blink/renderer/modules/mediastream/restriction_target.h" #include "third_party/blink/renderer/platform/bindings/exception_state.h" #include "third_party/blink/renderer/platform/testing/runtime_enabled_features_test_helpers.h" @@ -536,6 +538,45 @@ } #endif // !BUILDFLAG(IS_ANDROID) && !BUILDFLAG(IS_IOS) +class MockMediaPermission : public media::MediaPermission { + public: + MockMediaPermission() = default; + + void HasPermission(Type type, + PermissionStatusCB permission_status_cb) override { + bool has_permission = false; + if (type == Type::kAudioCapture) { + has_permission = has_microphone_permission_; + } else if (type == Type::kVideoCapture) { + has_permission = has_camera_permission_; + } + + std::move(permission_status_cb).Run(has_permission); + } + + void RequestPermission(Type type, + PermissionStatusCB permission_status_cb) override {} + + bool IsEncryptedMediaEnabled() override { return false; } + +#if BUILDFLAG(IS_WIN) + void IsHardwareSecureDecryptionAllowed( + IsHardwareSecureDecryptionAllowedCB cb) override {} +#endif // BUILDFLAG(IS_WIN) + + void SetCameraPermission(bool has_permission) { + has_camera_permission_ = has_permission; + } + + void SetMicrophonePermission(bool has_permission) { + has_microphone_permission_ = has_permission; + } + + private: + bool has_camera_permission_ = true; + bool has_microphone_permission_ = true; +}; + } // namespace class MediaDevicesTest : public PageTestBase { @@ -543,7 +584,8 @@ using MediaDeviceInfos = HeapVector<Member<MediaDeviceInfo>>; MediaDevicesTest() - : dispatcher_host_(std::make_unique<MockMediaDevicesDispatcherHost>()), + : platform_(std::make_unique<MockMediaPermission>()), + dispatcher_host_(std::make_unique<MockMediaDevicesDispatcherHost>()), device_infos_(MakeGarbageCollected<MediaDeviceInfos>()) {} MediaDevices* GetMediaDevices(LocalDOMWindow& window) { @@ -560,7 +602,9 @@ void OnListenerConnectionError() { listener_connection_error_ = true; } bool listener_connection_error() const { return listener_connection_error_; } - ScopedTestingPlatformSupport<TestingPlatformSupport>& platform() { + ScopedTestingPlatformSupport<MediaPermissionTestingPlatform, + std::unique_ptr<media::MediaPermission>>& + platform() { return platform_; } @@ -618,8 +662,22 @@ tester.Value().V8Value()); } + void SetCameraPermission(bool has_permission) { + static_cast<MockMediaPermission*>( + platform()->GetWebRTCMediaPermission(nullptr)) + ->SetCameraPermission(has_permission); + } + + void SetMicrophonePermission(bool has_permission) {
Loading diff…
Regression Test / PoC
shipped with the fix
diff --git a/third_party/blink/renderer/modules/mediastream/media_devices_test.cc b/third_party/blink/renderer/modules/mediastream/media_devices_test.cc
index bcf42030..3ec93f4 100644
--- a/third_party/blink/renderer/modules/mediastream/media_devices_test.cc
+++ b/third_party/blink/renderer/modules/mediastream/media_devices_test.cc
@@ -9,6 +9,7 @@
#include "base/test/metrics/histogram_tester.h"
#include "build/build_config.h"
+#include "media/base/media_permission.h"
#include "media/base/output_device_info.h"
#include "media/base/video_types.h"
#include "media/capture/mojom/video_capture_types.mojom.h"
@@ -50,6 +51,7 @@
#include "third_party/blink/renderer/modules/mediastream/crop_target.h"
#include "third_party/blink/renderer/modules/mediastream/input_device_info.h"
#include "third_party/blink/renderer/modules/mediastream/media_device_info.h"
+#include "third_party/blink/renderer/modules/mediastream/media_permission_testing_platform.h"
#include "third_party/blink/renderer/modules/mediastream/restriction_target.h"
#include "third_party/blink/renderer/platform/bindings/exception_state.h"
#include "third_party/blink/renderer/platform/testing/runtime_enabled_features_test_helpers.h"
@@ -536,6 +538,45 @@
}
#endif // !BUILDFLAG(IS_ANDROID) && !BUILDFLAG(IS_IOS)
+class MockMediaPermission : public media::MediaPermission {
+ public:
+ MockMediaPermission() = default;
+
+ void HasPermission(Type type,
+ PermissionStatusCB permission_status_cb) override {
+ bool has_permission = false;
+ if (type == Type::kAudioCapture) {
+ has_permission = has_microphone_permission_;
+ } else if (type == Type::kVideoCapture) {
+ has_permission = has_camera_permission_;
+ }
+
+ std::move(permission_status_cb).Run(has_permission);
+ }
+
+ void RequestPermission(Type type,
+ PermissionStatusCB permission_status_cb) override {}
+
+ bool IsEncryptedMediaEnabled() override { return false; }
+
+#if BUILDFLAG(IS_WIN)
+ void IsHardwareSecureDecryptionAllowed(
+ IsHardwareSecureDecryptionAllowedCB cb) override {}
+#endif // BUILDFLAG(IS_WIN)
+
+ void SetCameraPermission(bool has_permission) {
+ has_camera_permission_ = has_permission;
+ }
+
+ void SetMicrophonePermission(bool has_permission) {
+ has_microphone_permission_ = has_permission;
+ }
+
+ private:
+ bool has_camera_permission_ = true;
+ bool has_microphone_permission_ = true;
+};
+
} // namespace
class MediaDevicesTest : public PageTestBase {
@@ -543,7 +584,8 @@
using MediaDeviceInfos = HeapVector<Member<MediaDeviceInfo>>;
MediaDevicesTest()
- : dispatcher_host_(std::make_unique<MockMediaDevicesDispatcherHost>()),
+ : platform_(std::make_unique<MockMediaPermission>()),
+ dispatcher_host_(std::make_unique<MockMediaDevicesDispatcherHost>()),
device_infos_(MakeGarbageCollected<MediaDeviceInfos>()) {}
MediaDevices* GetMediaDevices(LocalDOMWindow& window) {
@@ -560,7 +602,9 @@
void OnListenerConnectionError() { listener_connection_error_ = true; }
bool listener_connection_error() const { return listener_connection_error_; }
- ScopedTestingPlatformSupport<TestingPlatformSupport>& platform() {
+ ScopedTestingPlatformSupport<MediaPermissionTestingPlatform,
+ std::unique_ptr<media::MediaPermission>>&
+ platform() {
return platform_;
}
@@ -618,8 +662,22 @@
tester.Value().V8Value());
}
+ void SetCameraPermission(bool has_permission) {
+ static_cast<MockMediaPermission*>(
+ platform()->GetWebRTCMediaPermission(nullptr))
+ ->SetCameraPermission(has_permission);
+ }
+
+ void SetMicrophonePermission(bool has_permission) {
+ static_cast<MockMediaPermission*>(
+ platform()->GetWebRTCMediaPermission(nullptr))
+ ->SetMicrophonePermission(has_permission);
+ }
+
private:
- ScopedTestingPlatformSupport<TestingPlatformSupport> platform_;
+ ScopedTestingPlatformSupport<MediaPermissionTestingPlatform,
+ std::unique_ptr<media::MediaPermission>>
+ platform_;
std::unique_ptr<MockMediaDevicesDispatcherHost> dispatcher_host_;
Persistent<MediaDeviceInfos> device_infos_;
bool listener_connection_error_ = false;
@@ -816,6 +874,46 @@
NotifyDeviceChanges();
}
+TEST_F(MediaDevicesTest, ObserveDeviceChangeEventPermissions) {
+ if (!RuntimeEnabledFeatures::OnDeviceChangeEnabled()) {
+ return;
+ }
+ StrictMock<MockDeviceChangeEventListener>* event_listener =
+ MakeGarbageCollected<StrictMock<MockDeviceChangeEventListener>>();
+ AddDeviceChangeListener(event_listener);
+
+ SetCameraPermission(false);
+ SetMicrophonePermission(true);
+
+ EXPECT_CALL(*event_listener, Invoke(_, _)).Times(0);
+ dispatcher_host().VideoInputDevices().begin()->device_id = "new_device_id";
+ NotifyDeviceChanges();
+
+ EXPECT_CALL(*event_listener, Invoke(_, _));
+ dispatcher_host().AudioInputDevices().begin()->device_id = "new_device_id";
+ NotifyDeviceChanges();
+
+ SetCameraPermission(true);
+ SetMicrophonePermission(false);
+
+ EXPECT_CALL(*event_listener, Invoke(_, _));
+ dispatcher_host().VideoInputDevices().begin()->device_id = "new_device_id_2";
+ NotifyDeviceChanges();
+
+ EXPECT_CALL(*event_listener, Invoke(_, _)).Times(0);
+ dispatcher_host().AudioInputDevices().begin()->device_id = "new_device_id_2";
+ NotifyDeviceChanges();
+
+ SetCameraPermission(false);
+ SetMicrophonePermission(false);
+
+ EXPECT_CALL(*event_listener, Invoke(_, _)).Times(0);
+ dispatcher_host().VideoInputDevices().begin()->device_id = "new_device_id_3";
+ NotifyDeviceChanges();
+ dispatcher_host().AudioInputDevices().begin()->device_id = "new_device_id_3";
+ NotifyDeviceChanges();
+}
+
TEST_F(MediaDevicesTest, SetCaptureHandleConfigEmpty) {
V8TestingScope scope;
auto* media_devices = GetMediaDevices(*GetDocument().domWindow());
Loading diff…
Original Bug Report
reported by [email protected]
devicechange event leaks for macbook's internal camera in sandboxed documents.
Steps to reproduce the problem
https://jsfiddle.net/wyfvm0jd/
- Connect a macbook with an internal camera (maybe affects other manufacturers, not sure) to a second display.
- In a sandboxed document with only “allow-scripts”, listen for the
navigator.mediaDevices.ondevicechangeevent. - Close the lid of the macbook.
Problem Description
The event is not supposed to fire in this case. The document is not allowed to access the camera details. This can be checked by requesting a new MediaStream from the camera, it’s correctly blocked. Note that if an external camera is connected (I tested with an USB one), then the event doesn’t fire, even for the internal one.
Summary
devicechange event leaks for macbook’s internal camera in sandboxed documents.
Custom Questions
Reporter credit:
Kaiido
Additional Data
Category: Security
Chrome Channel: Canary
Regression: N/A
References
On This Page