Title: [290330] trunk
- Revision
- 290330
- Author
- [email protected]
- Date
- 2022-02-22 12:54:54 -0800 (Tue, 22 Feb 2022)
Log Message
null ptr deref via WebXRSystem::requestSession
https://bugs.webkit.org/show_bug.cgi?id=235916
Patch by Frédéric Wang <[email protected]> on 2022-02-22
Reviewed by Dean Jackson.
Source/WebCore:
WebXRSystem::requestSession() null checks the document's global object via the method
Document::domWindow() and calls WebXRSystem::inlineSessionRequestIsAllowedForGlobalObject()
which dereferences the global object via the method ScriptExecutionContext::globalObject().
The former is just getting a raw pointer from Document::m_domWindow which (once set) remains
non-null until the document is destroyed. The latter instead gets the DOM window via the
document's FrameDestructionObserver::m_frame which is null when document is detached from the
frame. Hence the two methods may disagree, leading to a null ptr deref in
WebXRSystem::inlineSessionRequestIsAllowedForGlobalObject(). This patch works around that
issue by explicitly null checking the result of Document::::globalObject(). Additionally, it
makes the document (and its m_domWindow member) protected earlier in
WebXRSystem::requestSession() i.e. before passing them to potentially complex subroutines
immersiveSessionRequestIsAllowedForGlobalObject() and
inlineSessionRequestIsAllowedForGlobalObject().
Test: webxr/xr-requestSession-crash.html
* Modules/webxr/WebXRSystem.cpp:
(WebCore::WebXRSystem::inlineSessionRequestIsAllowedForGlobalObject const): null-check
document::globalObject() before deferencing it.
(WebCore::WebXRSystem::requestSession): protect document (and its m_domWindow member).
LayoutTests:
Add non-regression test.
* webxr/xr-requestSession-crash-expected.txt: Added.
* webxr/xr-requestSession-crash.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (290329 => 290330)
--- trunk/LayoutTests/ChangeLog 2022-02-22 20:49:12 UTC (rev 290329)
+++ trunk/LayoutTests/ChangeLog 2022-02-22 20:54:54 UTC (rev 290330)
@@ -1,3 +1,15 @@
+2022-02-22 Frédéric Wang <[email protected]>
+
+ null ptr deref via WebXRSystem::requestSession
+ https://bugs.webkit.org/show_bug.cgi?id=235916
+
+ Reviewed by Dean Jackson.
+
+ Add non-regression test.
+
+ * webxr/xr-requestSession-crash-expected.txt: Added.
+ * webxr/xr-requestSession-crash.html: Added.
+
2022-02-22 Philippe Normand <[email protected]>
[GStreamer] Initial MediaRecorder implementation
Added: trunk/LayoutTests/webxr/xr-requestSession-crash-expected.txt (0 => 290330)
--- trunk/LayoutTests/webxr/xr-requestSession-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/webxr/xr-requestSession-crash-expected.txt 2022-02-22 20:54:54 UTC (rev 290330)
@@ -0,0 +1 @@
+This test passes if it does not crash.
Added: trunk/LayoutTests/webxr/xr-requestSession-crash.html (0 => 290330)
--- trunk/LayoutTests/webxr/xr-requestSession-crash.html (rev 0)
+++ trunk/LayoutTests/webxr/xr-requestSession-crash.html 2022-02-22 20:54:54 UTC (rev 290330)
@@ -0,0 +1,23 @@
+<p>This test passes if it does not crash.</p>
+<script>
+ if (window.internals)
+ internals.settings.setWebXREnabled(true);
+ if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.waitUntilDone();
+ }
+ (async () => {
+ new Image().src = '';
+ for (let i = 0; i < 100; i++) {
+ if (window.caches)
+ await caches.has(i.toString());
+ navigator.xr.requestSession('inline', {optionalFeatures: [{}]}).
+ catch((e) => {
+ if (e instanceof DOMException && e.name === 'SecurityError') return;
+ throw e;
+ });
+ }
+ })();
+ if (window.testRunner)
+ testRunner.notifyDone();
+</script>
Modified: trunk/Source/WebCore/ChangeLog (290329 => 290330)
--- trunk/Source/WebCore/ChangeLog 2022-02-22 20:49:12 UTC (rev 290329)
+++ trunk/Source/WebCore/ChangeLog 2022-02-22 20:54:54 UTC (rev 290330)
@@ -1,3 +1,30 @@
+2022-02-22 Frédéric Wang <[email protected]>
+
+ null ptr deref via WebXRSystem::requestSession
+ https://bugs.webkit.org/show_bug.cgi?id=235916
+
+ Reviewed by Dean Jackson.
+ WebXRSystem::requestSession() null checks the document's global object via the method
+ Document::domWindow() and calls WebXRSystem::inlineSessionRequestIsAllowedForGlobalObject()
+ which dereferences the global object via the method ScriptExecutionContext::globalObject().
+ The former is just getting a raw pointer from Document::m_domWindow which (once set) remains
+ non-null until the document is destroyed. The latter instead gets the DOM window via the
+ document's FrameDestructionObserver::m_frame which is null when document is detached from the
+ frame. Hence the two methods may disagree, leading to a null ptr deref in
+ WebXRSystem::inlineSessionRequestIsAllowedForGlobalObject(). This patch works around that
+ issue by explicitly null checking the result of Document::::globalObject(). Additionally, it
+ makes the document (and its m_domWindow member) protected earlier in
+ WebXRSystem::requestSession() i.e. before passing them to potentially complex subroutines
+ immersiveSessionRequestIsAllowedForGlobalObject() and
+ inlineSessionRequestIsAllowedForGlobalObject().
+
+ Test: webxr/xr-requestSession-crash.html
+
+ * Modules/webxr/WebXRSystem.cpp:
+ (WebCore::WebXRSystem::inlineSessionRequestIsAllowedForGlobalObject const): null-check
+ document::globalObject() before deferencing it.
+ (WebCore::WebXRSystem::requestSession): protect document (and its m_domWindow member).
+
2022-02-22 Chris Dumez <[email protected]>
Clean up / optimize even more call sites constructing vectors
Modified: trunk/Source/WebCore/Modules/webxr/WebXRSystem.cpp (290329 => 290330)
--- trunk/Source/WebCore/Modules/webxr/WebXRSystem.cpp 2022-02-22 20:49:12 UTC (rev 290329)
+++ trunk/Source/WebCore/Modules/webxr/WebXRSystem.cpp 2022-02-22 20:54:54 UTC (rev 290330)
@@ -221,7 +221,7 @@
auto isEmptyOrViewer = [&document](const JSFeatureList& features) {
if (features.isEmpty())
return true;
- if (features.size() == 1) {
+ if (features.size() == 1 && document.globalObject()) {
auto feature = parseEnumeration<XRReferenceSpaceType>(*document.globalObject(), features.first());
if (feature == XRReferenceSpaceType::Viewer)
return true;
@@ -400,7 +400,8 @@
// 2. Let immersive be true if mode is an immersive session mode, and false otherwise.
// 3. Let global object be the relevant Global object for the XRSystem on which this method was invoked.
bool immersive = mode == XRSessionMode::ImmersiveAr || mode == XRSessionMode::ImmersiveVr;
- auto* globalObject = document.domWindow();
+ Ref protectedDocument { document };
+ auto* globalObject = protectedDocument->domWindow();
if (!globalObject) {
promise.reject(Exception { InvalidAccessError});
return;
@@ -427,7 +428,7 @@
// 5.2 Let optionalFeatures be options' optionalFeatures.
// 5.3 Set device to the result of obtaining the current device for mode, requiredFeatures, and optionalFeatures.
// 5.4 Queue a task to perform the following steps:
- obtainCurrentDevice(mode, init.requiredFeatures, init.optionalFeatures, [this, protectedDocument = Ref { document }, immersive, init, mode, promise = WTFMove(promise)](auto* device) mutable {
+ obtainCurrentDevice(mode, init.requiredFeatures, init.optionalFeatures, [this, protectedDocument, immersive, init, mode, promise = WTFMove(promise)](auto* device) mutable {
auto rejectPromiseWithNotSupportedError = makeScopeExit([&]() {
promise.reject(Exception { NotSupportedError });
m_pendingImmersiveSession = false;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
