Regression(r275668) Potential null pointer deref in AudioParam::exponentialRampToValueAtTime(float, double)

cdumez · cdumez · commit d315a91f1c9b · 2021-04-10T22:41:18.000Z
https://bugs.webkit.org/show_bug.cgi?id=224400 <rdar://76450376> Reviewed by Ryosuke Niwa. Source/WebCore: In r275668, I added null-checks for the AudioContext in AudioParam, now that it holds a WeakPtr to its context. However, I missed a null-check in AudioParam::exponentialRampToValueAtTime(). This patch adds the missing check. Test: webaudio/AudioParam/audioparam-exponentialRampToValueAtTime-nocontext-crash.html * Modules/webaudio/AudioParam.cpp: LayoutTests: Add layout test coverage. * webaudio/AudioParam/audioparam-exponentialRampToValueAtTime-nocontext-crash-expected.txt: Added. * webaudio/AudioParam/audioparam-exponentialRampToValueAtTime-nocontext-crash.html: Added. Canonical link: https://commits.webkit.org/236375@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@275804 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2021-04-10  Chris Dumez  <cdumez@apple.com>
+
+        Regression(r275668) Potential null pointer deref in AudioParam::exponentialRampToValueAtTime(float, double)
+        https://bugs.webkit.org/show_bug.cgi?id=224400
+        <rdar://76450376>
+
+        Reviewed by Ryosuke Niwa.
+
+        Add layout test coverage.
+
+        * webaudio/AudioParam/audioparam-exponentialRampToValueAtTime-nocontext-crash-expected.txt: Added.
+        * webaudio/AudioParam/audioparam-exponentialRampToValueAtTime-nocontext-crash.html: Added.
+
 2021-04-10  Chris Dumez  <cdumez@apple.com>
 
         [ macOS ] imported/w3c/web-platform-tests/webaudio/the-audio-api/the-analysernode-interface/realtimeanalyser-fft-scaling.html is a flakey text failure
diff --git a/LayoutTests/webaudio/AudioParam/audioparam-exponentialRampToValueAtTime-nocontext-crash-expected.txt b/LayoutTests/webaudio/AudioParam/audioparam-exponentialRampToValueAtTime-nocontext-crash-expected.txt
@@ -0,0 +1,10 @@
+Make sure we don't crash when calling linearRampToValueAtTime() on an AudioParam that lost its AudioContext.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Did not crash
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/webaudio/AudioParam/audioparam-exponentialRampToValueAtTime-nocontext-crash.html b/LayoutTests/webaudio/AudioParam/audioparam-exponentialRampToValueAtTime-nocontext-crash.html
@@ -0,0 +1,14 @@
+<DOCTYPE html>
+<html>
+<body>
+<script src="../../resources/js-test.js"></script>
+<script>
+description("Make sure we don't crash when calling linearRampToValueAtTime() on an AudioParam that lost its AudioContext.");
+
+let audioParam = new OfflineAudioContext(1, 1, 3000).listener.forwardX;
+gc();
+audioParam.exponentialRampToValueAtTime(1, 0);
+testPassed("Did not crash");
+</script>
+</body>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,19 @@
+2021-04-10  Chris Dumez  <cdumez@apple.com>
+
+        Regression(r275668) Potential null pointer deref in AudioParam::exponentialRampToValueAtTime(float, double)
+        https://bugs.webkit.org/show_bug.cgi?id=224400
+        <rdar://76450376>
+
+        Reviewed by Ryosuke Niwa.
+
+        In r275668, I added null-checks for the AudioContext in AudioParam, now that it holds a WeakPtr to its
+        context. However, I missed a null-check in AudioParam::exponentialRampToValueAtTime(). This patch adds
+        the missing check.
+
+        Test: webaudio/AudioParam/audioparam-exponentialRampToValueAtTime-nocontext-crash.html
+
+        * Modules/webaudio/AudioParam.cpp:
+
 2021-04-10  Chris Dumez  <cdumez@apple.com>
 
         [ macOS ] 3 webaudio/OfflineAudioContext/ layout-tests are flakey text failures
diff --git a/Source/WebCore/Modules/webaudio/AudioParam.cpp b/Source/WebCore/Modules/webaudio/AudioParam.cpp
@@ -182,6 +182,9 @@ ExceptionOr<AudioParam&> AudioParam::linearRampToValueAtTime(float value, double
 
 ExceptionOr<AudioParam&> AudioParam::exponentialRampToValueAtTime(float value, double endTime)
 {
+    if (!context())
+        return *this;
+
     if (!value)
         return Exception { RangeError, "value cannot be 0"_s };
     if (endTime < 0)

Original file line number	Diff line number	Diff line change
`@@ -182,6 +182,9 @@ ExceptionOr<AudioParam&> AudioParam::linearRampToValueAtTime(float value, double`
`182`	`182`
`183`	`183`	`ExceptionOr<AudioParam&> AudioParam::exponentialRampToValueAtTime(float value, double endTime)`
`184`	`184`	`{`
	`185`	`+ if (!context())`
	`186`	`+ return *this;`
	`187`	`+`
`185`	`188`	`if (!value)`
`186`	`189`	`return Exception { RangeError, "value cannot be 0"_s };`
`187`	`190`	`if (endTime < 0)`