Skip to content

Commit a198242

Browse files
FrankLi-MSFTmibrunin
authored andcommitted
[Backport] CVE-2024-11112: Use after free in Media
Manual cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/5910432: avoid heap-use-after-free in OverlayStateObserverImpl::Create In PS38 of https://chromium-review.googlesource.com/c/chromium/src/+/3403933/37..38, `scoped_refptr<OverlayStateServiceProvider>` parameter of `OverlayStateObserverImpl::Create()' was changed into `OverlayStateServiceProvider*`. This causes the heap-use-after-free issue described in https://issuetracker.google.com/issues/354824998. This CL is to follow existing code pattern of producing `scoped_refptr` in `RenderThreadImpl::GetStreamTextureFactory()`/ `RenderThreadImpl::GetDCOMPTextureFactor()` instead of returning a raw pointer in `RenderThreadImpl::GetOverlayStateServiceProvider()`. Bug: 354824998 Change-Id: Ic188bf18b28a5f024488bd92fb11f6fca6c5d05a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5910432 Reviewed-by: Dale Curtis <[email protected]> Reviewed-by: Daniel Cheng <[email protected]> Commit-Queue: Frank Li <[email protected]> Cr-Commit-Position: refs/heads/main@{#1368423} Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/607607 Reviewed-by: Anu Aliyas <[email protected]>
1 parent 4b6f666 commit a198242

File tree

5 files changed

+18
-9
lines changed

5 files changed

+18
-9
lines changed

chromium/content/renderer/media/media_factory.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -690,7 +690,7 @@ MediaFactory::CreateRendererFactorySelector(
690690

691691
media::ObserveOverlayStateCB observe_overlay_state_cb =
692692
base::BindRepeating(&OverlayStateObserverImpl::Create,
693-
render_thread->GetOverlayStateServiceProvider());
693+
base::RetainedRef(render_thread->GetOverlayStateServiceProvider()));
694694

695695
factory_selector->AddFactory(
696696
RendererType::kMediaFoundation,

chromium/content/renderer/media/win/overlay_state_observer_impl.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ OverlayStateObserverImpl::Create(
1616
StateChangedCB state_changed_cb) {
1717
if (overlay_state_service_provider) {
1818
return base::WrapUnique(new OverlayStateObserverImpl(
19-
overlay_state_service_provider, mailbox, state_changed_cb));
19+
overlay_state_service_provider, mailbox, std::move(state_changed_cb)));
2020
}
2121
return nullptr;
2222
}

chromium/content/renderer/media/win/overlay_state_service_provider.h

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,19 @@ class GpuChannelHost;
1515

1616
namespace content {
1717

18-
class OverlayStateServiceProvider {
18+
class OverlayStateServiceProvider
19+
: public base::RefCountedThreadSafe<OverlayStateServiceProvider> {
1920
public:
2021
virtual bool RegisterObserver(
2122
mojo::PendingRemote<gpu::mojom::OverlayStateObserver> pending_remote,
2223
const gpu::Mailbox& mailbox) = 0;
24+
25+
protected:
26+
friend class base::RefCountedThreadSafe<OverlayStateServiceProvider>;
27+
OverlayStateServiceProvider() = default;
28+
OverlayStateServiceProvider(const OverlayStateServiceProvider&) = delete;
29+
OverlayStateServiceProvider& operator=(const OverlayStateServiceProvider&) =
30+
delete;
2331
virtual ~OverlayStateServiceProvider() = default;
2432
};
2533

@@ -29,7 +37,6 @@ class OverlayStateServiceProviderImpl : public OverlayStateServiceProvider {
2937
public:
3038
explicit OverlayStateServiceProviderImpl(
3139
scoped_refptr<gpu::GpuChannelHost> channel);
32-
~OverlayStateServiceProviderImpl() override;
3340

3441
bool RegisterObserver(
3542
mojo::PendingRemote<gpu::mojom::OverlayStateObserver> pending_remote,
@@ -43,6 +50,7 @@ class OverlayStateServiceProviderImpl : public OverlayStateServiceProvider {
4350
delete;
4451
OverlayStateServiceProviderImpl& operator=(
4552
const OverlayStateServiceProviderImpl&) = delete;
53+
~OverlayStateServiceProviderImpl() override;
4654

4755
scoped_refptr<gpu::GpuChannelHost> channel_;
4856
};

chromium/content/renderer/render_thread_impl.cc

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1268,7 +1268,7 @@ scoped_refptr<DCOMPTextureFactory> RenderThreadImpl::GetDCOMPTextureFactory() {
12681268
return dcomp_texture_factory_;
12691269
}
12701270

1271-
OverlayStateServiceProvider*
1271+
scoped_refptr<OverlayStateServiceProvider>
12721272
RenderThreadImpl::GetOverlayStateServiceProvider() {
12731273
DCHECK(IsMainThread());
12741274
// Only set 'overlay_state_service_provider_' if Media Foundation for clear
@@ -1282,11 +1282,12 @@ RenderThreadImpl::GetOverlayStateServiceProvider() {
12821282
return nullptr;
12831283
}
12841284
overlay_state_service_provider_ =
1285-
std::make_unique<OverlayStateServiceProviderImpl>(std::move(channel));
1285+
base::MakeRefCounted<OverlayStateServiceProviderImpl>(
1286+
std::move(channel));
12861287
}
12871288
}
12881289

1289-
return overlay_state_service_provider_.get();
1290+
return overlay_state_service_provider_;
12901291
}
12911292
#endif // BUILDFLAG(IS_WIN)
12921293

chromium/content/renderer/render_thread_impl.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ class CONTENT_EXPORT RenderThreadImpl
265265
// The OverlayStateService is only available where Media Foundation for
266266
// clear is supported, otherwise GetOverlayStateServiceProvider will return
267267
// nullptr.
268-
OverlayStateServiceProvider* GetOverlayStateServiceProvider();
268+
scoped_refptr<OverlayStateServiceProvider> GetOverlayStateServiceProvider();
269269
#endif
270270

271271
blink::WebVideoCaptureImplManager* video_capture_impl_manager() const {
@@ -530,7 +530,7 @@ class CONTENT_EXPORT RenderThreadImpl
530530

531531
#if BUILDFLAG(IS_WIN)
532532
scoped_refptr<DCOMPTextureFactory> dcomp_texture_factory_;
533-
std::unique_ptr<OverlayStateServiceProviderImpl>
533+
scoped_refptr<OverlayStateServiceProviderImpl>
534534
overlay_state_service_provider_;
535535
#endif
536536

0 commit comments

Comments
 (0)