Null pointer dereference on MediaStreamAudioSourceNode destructor

Ian Gilbert · webkit-commit-queue · commit daa5f8371bb1 · 2021-04-15T22:40:41.000Z
https://bugs.webkit.org/show_bug.cgi?id=224573 Patch by Ian Gilbert <iang@apple.com> on 2021-04-15 Reviewed by Eric Carlson. Source/WebCore: Add null pointer check in case the capture source no longer exists. Test: webaudio/destroy-audio-source-node-with-no-track-crash.html * platform/mediastream/mac/MediaStreamTrackAudioSourceProviderCocoa.cpp: (WebCore::MediaStreamTrackAudioSourceProviderCocoa::hasNewClient): LayoutTests: Add regression test. * webaudio/destroy-audio-source-node-with-no-track-crash-expected.txt: Added. * webaudio/destroy-audio-source-node-with-no-track-crash.html: Added. Canonical link: https://commits.webkit.org/236592@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@276079 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,15 @@
+2021-04-15  Ian Gilbert  <iang@apple.com>
+
+        Null pointer dereference on MediaStreamAudioSourceNode destructor
+        https://bugs.webkit.org/show_bug.cgi?id=224573
+
+        Reviewed by Eric Carlson.
+
+        Add regression test.
+
+        * webaudio/destroy-audio-source-node-with-no-track-crash-expected.txt: Added.
+        * webaudio/destroy-audio-source-node-with-no-track-crash.html: Added.
+
 2021-04-15  Julian Gonzalez  <julian_a_gonzalez@apple.com>
 
         Crash in ReplaceSelectionCommand::mergeEndIfNeeded()
diff --git a/LayoutTests/webaudio/destroy-audio-source-node-with-no-track-crash-expected.txt b/LayoutTests/webaudio/destroy-audio-source-node-with-no-track-crash-expected.txt
@@ -0,0 +1,3 @@
+Test passes if it does not crash.
+
+PASS
diff --git a/LayoutTests/webaudio/destroy-audio-source-node-with-no-track-crash.html b/LayoutTests/webaudio/destroy-audio-source-node-with-no-track-crash.html
@@ -0,0 +1,18 @@
+<p>Test passes if it does not crash.</p>
+<script>
+  if (window.testRunner)
+    testRunner.dumpAsText();
+
+  let audioContext = new AudioContext();
+  let mediaStreamAudioDestinationNode = new MediaStreamAudioDestinationNode(audioContext);
+  new MediaStreamAudioSourceNode(audioContext, {mediaStream: mediaStreamAudioDestinationNode.stream});
+  
+  let track0 = mediaStreamAudioDestinationNode.stream.getAudioTracks()[0];
+  track0.stop();
+  mediaStreamAudioDestinationNode.stream.removeTrack(track0);
+  track0 = undefined;
+  
+  GCController.collect();
+  
+  document.write("PASS");
+</script>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,17 @@
+2021-04-15  Ian Gilbert  <iang@apple.com>
+
+        Null pointer dereference on MediaStreamAudioSourceNode destructor
+        https://bugs.webkit.org/show_bug.cgi?id=224573
+
+        Reviewed by Eric Carlson.
+
+        Add null pointer check in case the capture source no longer exists.
+
+        Test: webaudio/destroy-audio-source-node-with-no-track-crash.html
+
+        * platform/mediastream/mac/MediaStreamTrackAudioSourceProviderCocoa.cpp:
+        (WebCore::MediaStreamTrackAudioSourceProviderCocoa::hasNewClient):
+
 2021-04-15  Julian Gonzalez  <julian_a_gonzalez@apple.com>
 
         Crash in ReplaceSelectionCommand::mergeEndIfNeeded()
diff --git a/Source/WebCore/platform/mediastream/mac/MediaStreamTrackAudioSourceProviderCocoa.cpp b/Source/WebCore/platform/mediastream/mac/MediaStreamTrackAudioSourceProviderCocoa.cpp
@@ -55,7 +55,8 @@ void MediaStreamTrackAudioSourceProviderCocoa::hasNewClient(AudioSourceProviderC
 
     m_connected = shouldBeConnected;
     if (!client) {
-        m_captureSource->removeObserver(*this);
+        if (m_captureSource)
+            m_captureSource->removeObserver(*this);
         m_source->removeAudioSampleObserver(*this);
         return;
     }

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+Test passes if it does not crash.`
	`2`	`+`
	`3`	`+PASS`
Original file line number	Diff line number	Diff line change
`@@ -55,7 +55,8 @@ void MediaStreamTrackAudioSourceProviderCocoa::hasNewClient(AudioSourceProviderC`
`55`	`55`
`56`	`56`	`m_connected = shouldBeConnected;`
`57`	`57`	`if (!client) {`
`58`		`- m_captureSource->removeObserver(*this);`
	`58`	`+ if (m_captureSource)`
	`59`	`+ m_captureSource->removeObserver(*this);`
`59`	`60`	`m_source->removeAudioSampleObserver(*this);`
`60`	`61`	`return;`
`61`	`62`	`}`