PatchGap
Chrome · Media Stream

CVE-2025-1921

Logic Error in Media Stream
Medium chrome Logic Error 📄 Reporter bug report 🔧 Commit mapped

Overview

Medium
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactInappropriate Implementation in Media Stream
DescriptionInappropriate Implementation in Media Stream
ComponentMedia Stream
Bug ClassLogic Error
Tracker387583503
Fix commit5182f8756d86 (chromium/src) +185/-3
CISA KEVNot listed
CreditedKaiido
Disclosed2025-03-04

Changed Functions

FunctionChangeNotes
switch
third_party/blink/renderer/modules/mediastream/media_devices.cc
 modified
if
third_party/blink/renderer/modules/mediastream/media_devices.cc
 modified
MockMediaPermission
third_party/blink/renderer/modules/mediastream/media_devices_test.cc
 modified
if
third_party/blink/renderer/modules/mediastream/media_devices_test.cc
 modified
MediaDevicesTest
third_party/blink/renderer/modules/mediastream/media_devices_test.cc
 modified
platform
third_party/blink/renderer/modules/mediastream/media_devices_test.cc
 modified

Files Changed

  • third_party/blink/renderer/modules/mediastream/BUILD.gn
  • third_party/blink/renderer/modules/mediastream/media_devices.cc
  • third_party/blink/renderer/modules/mediastream/media_devices.h
  • third_party/blink/renderer/modules/mediastream/media_devices_test.cc

Patch

view commit
From 5182f8756d86cb441d3445fcb628f961e74bd6ca Mon Sep 17 00:00:00 2001
From: Guido Urdaneta <guidou@chromium.org>
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 <agpalak@chromium.org>
Commit-Queue: Guido Urdaneta <guidou@chromium.org>
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 tr...@gmail.com

devicechange event leaks for macbook's internal camera in sandboxed documents.

Steps to reproduce the problem

https://jsfiddle.net/wyfvm0jd/

  1. Connect a macbook with an internal camera (maybe affects other manufacturers, not sure) to a second display.
  2. In a sandboxed document with only “allow-scripts”, listen for the navigator.mediaDevices.ondevicechange event.
  3. 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

View on issue tracker
Links in the report
On This Page