Fix UAF in MediaPlayerPrivateMediaStreamAVFObjC::processNewVideoFrame

https://bugs.webkit.org/show_bug.cgi?id=256173 rdar://108504399 Reviewed by Jer Noble and Youenn Fablet. This change fixes the heap UAF on MediaPlayer element by protecting the MediaPlayer object when executing callbacks/deferred tasks on the mainThread, so that MediaPlayerPrivateMediaStreamAVFObjC remains valid. * Source/WebCore/html/HTMLMediaElement.cpp: * Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm: (WebCore::MediaPlayerPrivateMediaStreamAVFObjC::processNewVideoFrame): (WebCore::MediaPlayerPrivateMediaStreamAVFObjC::scheduleDeferredTask): * LayoutTests/fast/media/media-player-uaf-expected.txt: Added. * LayoutTests/fast/media/media-player-uaf.html: Added. Originally-landed-as: 259548.728@safari-7615-branch (4206d48). rdar://108504399 Canonical link: https://commits.webkit.org/266444@main
WebKit · Jul 31, 2023 · ce446a1 · ce446a1
1 parent 35a6bec
commit ce446a1
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 8 deletions.
diff --git a/LayoutTests/fast/media/media-player-uaf-expected.txt b/LayoutTests/fast/media/media-player-uaf-expected.txt
@@ -0,0 +1 @@
+
diff --git a/LayoutTests/fast/media/media-player-uaf.html b/LayoutTests/fast/media/media-player-uaf.html
@@ -0,0 +1,12 @@
+<script>
+  onload = async () => {
+    if (window.testRunner)
+       testRunner.dumpAsText();
+    let video0 = document.createElement('video');
+    video0.srcObject = await navigator.mediaDevices.getUserMedia({video: true});
+    let textTrack = video0.addTextTrack('subtitles', '');
+    textTrack.addCue(new VTTCue(0, 1, 'a'));
+    await caches.open('x');
+    video0.src = 'data:video/mp4;';
+  };
+</script>
diff --git a/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm b/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm
@@ -286,19 +286,16 @@ void getSupportedTypes(HashSet<String, ASCIICaseInsensitiveHash>& types) const f
             Locker locker { m_currentVideoFrameLock };
             m_currentVideoFrame = &videoFrame;
         }
-        callOnMainThread([weakThis = WeakPtr { *this }, metadata, presentationTime]() mutable {
-            if (!weakThis)
-                return;
-
+        scheduleDeferredTask([this, metadata, presentationTime]() mutable {
             RefPtr<VideoFrame> videoFrame;
             {
-                Locker locker { weakThis->m_currentVideoFrameLock };
-                videoFrame = WTFMove(weakThis->m_currentVideoFrame);
+                Locker locker { m_currentVideoFrameLock };
+                videoFrame = WTFMove(m_currentVideoFrame);
             }
             if (!videoFrame)
                 return;
 
-            weakThis->processNewVideoFrame(*videoFrame, metadata, presentationTime);
+            processNewVideoFrame(*videoFrame, metadata, presentationTime);
         });
         return;
     }
@@ -1141,7 +1138,7 @@ static inline CGAffineTransform videoTransformationMatrix(VideoFrame& videoFrame
     callOnMainThread([weakThis = WeakPtr { *this }, function = WTFMove(function)] {
         if (!weakThis)
             return;
-
+        auto protectedMediaPlayer = RefPtr { weakThis->m_player.get() };
         function();
     });
 }