CRASH in SourceBuffer::sourceBufferPrivateDidReceiveSample + 2169
authorjer.noble@apple.com <jer.noble@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Oct 2016 08:03:22 +0000 (08:03 +0000)
committerjer.noble@apple.com <jer.noble@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Oct 2016 08:03:22 +0000 (08:03 +0000)
https://bugs.webkit.org/show_bug.cgi?id=163735

Reviewed by Eric Carlson.

Source/WebCore:

Test: media/media-source/media-source-sample-wrong-track-id.html

When SourceBuffer receives a sample in sourceBufferPrivateDidReceiveSample() containing
a trackID not previously seen in an initialization segment, it creates a default TrackBuffer
object to contain that track's samples. One of the fields in TrackBuffer, description, is
normally filled out when an initialization segment is received, but with this default
TrackBuffer, it's still null when it's checked later in sourceBufferPrivateDidReceiveSample().

Rather than adding a null-check on trackBuffer.description, drop any sample that has a
trackID which was not present during a previous initialization segment.

* Modules/mediasource/SourceBuffer.cpp:
(WebCore::SourceBuffer::sourceBufferPrivateDidReceiveSample):

LayoutTests:

* media/media-source/media-source-sample-wrong-track-id-expected.txt: Added.
* media/media-source/media-source-sample-wrong-track-id.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@207661 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/media/media-source/media-source-sample-wrong-track-id-expected.txt [new file with mode: 0644]
LayoutTests/media/media-source/media-source-sample-wrong-track-id.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/Modules/mediasource/SourceBuffer.cpp

index a1d5329..8e62fe2 100644 (file)
@@ -1,3 +1,13 @@
+2016-10-21  Jer Noble  <jer.noble@apple.com>
+
+        CRASH in SourceBuffer::sourceBufferPrivateDidReceiveSample + 2169
+        https://bugs.webkit.org/show_bug.cgi?id=163735
+
+        Reviewed by Eric Carlson.
+
+        * media/media-source/media-source-sample-wrong-track-id-expected.txt: Added.
+        * media/media-source/media-source-sample-wrong-track-id.html: Added.
+
 2016-10-20  Zan Dobersek  <zdobersek@igalia.com>
 
         Import W3C EME tests
diff --git a/LayoutTests/media/media-source/media-source-sample-wrong-track-id-expected.txt b/LayoutTests/media/media-source/media-source-sample-wrong-track-id-expected.txt
new file mode 100644 (file)
index 0000000..88ea6db
--- /dev/null
@@ -0,0 +1,9 @@
+
+RUN(video.src = URL.createObjectURL(source))
+EVENT(sourceopen)
+RUN(sourceBuffer = source.addSourceBuffer("video/mock; codecs=mock"))
+Append a set of invalid, overlapping samples. Should not crash.
+RUN(sourceBuffer.appendBuffer(mediaSegment))
+EVENT(updateend)
+END OF TEST
+
diff --git a/LayoutTests/media/media-source/media-source-sample-wrong-track-id.html b/LayoutTests/media/media-source/media-source-sample-wrong-track-id.html
new file mode 100644 (file)
index 0000000..6d4d8f9
--- /dev/null
@@ -0,0 +1,40 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <title>media-source-sample-wrong-track-id</title>
+    <script src="mock-media-source.js"></script>
+    <script src="../video-test.js"></script>
+    <script>
+    var source;
+    var sourceBuffer;
+    var initSegment;
+    var mediaSegment;
+
+    if (window.internals)
+        internals.initializeMockMediaSource();
+
+    function runTest() {
+        findMediaElement();
+
+        source = new MediaSource();
+        waitForEventOn(source, 'sourceopen', sourceOpen, false, true);
+        run('video.src = URL.createObjectURL(source)');
+    }
+
+    function sourceOpen() {
+        run('sourceBuffer = source.addSourceBuffer("video/mock; codecs=mock")');
+        waitForEventOn(sourceBuffer, 'updateend', endTest);
+        consoleWrite('Append a set of invalid, overlapping samples. Should not crash.')
+        mediaSegment = concatenateSamples([
+            makeAInit(2, [makeATrack(1, 'mock', TRACK_KIND.AUDIO)]), 
+            makeASample(1, 1, 1, 2, SAMPLE_FLAG.SYNC, 0),
+            makeASample(1, 0, 2, 2, SAMPLE_FLAG.SYNC, 0),
+        ]);
+        run('sourceBuffer.appendBuffer(mediaSegment)');
+    }
+    </script>
+</head>
+<body onload="runTest()">
+    <video></video>
+</body>
+</html>
index c2f9605..93d79e6 100644 (file)
@@ -1,3 +1,24 @@
+2016-10-21  Jer Noble  <jer.noble@apple.com>
+
+        CRASH in SourceBuffer::sourceBufferPrivateDidReceiveSample + 2169
+        https://bugs.webkit.org/show_bug.cgi?id=163735
+
+        Reviewed by Eric Carlson.
+
+        Test: media/media-source/media-source-sample-wrong-track-id.html
+
+        When SourceBuffer receives a sample in sourceBufferPrivateDidReceiveSample() containing
+        a trackID not previously seen in an initialization segment, it creates a default TrackBuffer
+        object to contain that track's samples. One of the fields in TrackBuffer, description, is
+        normally filled out when an initialization segment is received, but with this default
+        TrackBuffer, it's still null when it's checked later in sourceBufferPrivateDidReceiveSample().
+
+        Rather than adding a null-check on trackBuffer.description, drop any sample that has a 
+        trackID which was not present during a previous initialization segment.
+
+        * Modules/mediasource/SourceBuffer.cpp:
+        (WebCore::SourceBuffer::sourceBufferPrivateDidReceiveSample):
+
 2016-10-20  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         [GTK] Configures but fails to link with ENABLE_OPENGL=OFF
index 61d7471..130d2fa 100644 (file)
@@ -1396,8 +1396,12 @@ void SourceBuffer::sourceBufferPrivateDidReceiveSample(SourceBufferPrivate*, Med
         // 1.5 Let track buffer equal the track buffer that the coded frame will be added to.
         AtomicString trackID = sample.trackID();
         auto it = m_trackBufferMap.find(trackID);
-        if (it == m_trackBufferMap.end())
-            it = m_trackBufferMap.add(trackID, TrackBuffer()).iterator;
+        if (it == m_trackBufferMap.end()) {
+            // The client managed to append a sample with a trackID not present in the initialization
+            // segment. This would be a good place to post an message to the developer console.
+            didDropSample();
+            return;
+        }
         TrackBuffer& trackBuffer = it->value;
 
         // 1.6 ↳ If last decode timestamp for track buffer is set and decode timestamp is less than last