0ct0pu5/ladybird
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 2

LibWeb: Don't crash when accessing property in detached Window object

Browse source 
After removing an iframe from the DOM, its contentWindow will be
detached from its browsing context, per spec.

Because the contentWindow is still accessible, we cannot assume that
Window objects always have an associated browsing context.

This needs to be fixed in the spec, but let's add a sensible null check
in the meantime.
This commit is contained in:
Andreas Kling 2024-03-11 09:35:35 +01:00
parent 2e0297d703
commit ad843b6e4a
Notes: sideshowbarker 2024-07-17 06:38:11 +09:00
5 changed files with 28 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
Tests/LibWeb/Text/expected/HTML/WindowProxy-Get-after-detaching-from-browsing-context.txt Normal file
View file

@ -0,0 +1,3 @@
null
PASS (didn't crash)

11
Tests/LibWeb/Text/input/HTML/WindowProxy-Get-after-detaching-from-browsing-context.html Normal file
View file

@ -0,0 +1,11 @@
<script src="../include.js"></script>
<script>
test(() => {
let iframe = document.querySelector("iframe")
let w = iframe.contentWindow;
println(w.getSelection());
iframe.remove();
println(w.getSelection());
println("PASS (didn't crash)");
})
</script><iframe></iframe>

14
Userland/Libraries/LibWeb/HTML/CrossOrigin/Reporting.cpp
View file

@ -12,8 +12,16 @@
namespace Web::HTML {
// https://html.spec.whatwg.org/multipage/origin.html#coop-check-access-report
void check_if_access_between_two_browsing_contexts_should_be_reported(BrowsingContext const& accessor, BrowsingContext const& accessed, JS::PropertyKey const& property_key, EnvironmentSettingsObject const& environment)
void check_if_access_between_two_browsing_contexts_should_be_reported(
BrowsingContext const& accessor,
BrowsingContext const* accessed,
JS::PropertyKey const& property_key,
EnvironmentSettingsObject const& environment)
{
// FIXME: Spec bug: https://github.com/whatwg/html/issues/10192
if (!accessed)
return;
// 1. If P is not a cross-origin accessible window property name, then return.
if (!is_cross_origin_accessible_window_property_name(property_key))
return;
@ -27,11 +35,11 @@ void check_if_access_between_two_browsing_contexts_should_be_reported(BrowsingCo
auto accessor_accessed_relationship = AccessorAccessedRelationship::None;
// 5. If accessed's top-level browsing context's opener browsing context is accessor or an ancestor of accessor, then set accessorAccessedRelationship to accessor is opener.
if (auto opener = accessed.top_level_browsing_context()->opener_browsing_context(); opener && (opener == &accessor || opener->is_ancestor_of(accessor)))
if (auto opener = accessed->top_level_browsing_context()->opener_browsing_context(); opener && (opener == &accessor || opener->is_ancestor_of(accessor)))
accessor_accessed_relationship = AccessorAccessedRelationship::AccessorIsOpener;
// 6. If accessor's top-level browsing context's opener browsing context is accessed or an ancestor of accessed, then set accessorAccessedRelationship to accessor is openee.
if (auto opener = accessor.top_level_browsing_context()->opener_browsing_context(); opener && (opener == &accessed || opener->is_ancestor_of(accessed)))
if (auto opener = accessor.top_level_browsing_context()->opener_browsing_context(); opener && (opener == accessed || opener->is_ancestor_of(*accessed)))
accessor_accessed_relationship = AccessorAccessedRelationship::AccessorIsOpenee;
// FIXME: 7. Queue violation reports for accesses, given accessorAccessedRelationship, accessor's top-level browsing context's active document's cross-origin opener policy, accessed's top-level browsing context's active document's cross-origin opener policy, accessor's active document's URL, accessed's active document's URL, accessor's top-level browsing context's initial URL, accessed's top-level browsing context's initial URL, accessor's active document's origin, accessed's active document's origin, accessor's top-level browsing context's opener origin at creation, accessed's top-level browsing context's opener origin at creation, accessor's top-level browsing context's active document's referrer, accessed's top-level browsing context's active document's referrer, P, and environment.

2
Userland/Libraries/LibWeb/HTML/CrossOrigin/Reporting.h
View file

@ -18,6 +18,6 @@ enum class AccessorAccessedRelationship {
None,
};
void check_if_access_between_two_browsing_contexts_should_be_reported(BrowsingContext const& accessor, BrowsingContext const& accessed, JS::PropertyKey const&, EnvironmentSettingsObject const&);
void check_if_access_between_two_browsing_contexts_should_be_reported(BrowsingContext const& accessor, BrowsingContext const* accessed, JS::PropertyKey const&, EnvironmentSettingsObject const&);
}

4
Userland/Libraries/LibWeb/HTML/WindowProxy.cpp
View file

@ -158,7 +158,7 @@ JS::ThrowCompletionOr<JS::Value> WindowProxy::internal_get(JS::PropertyKey const
// 1. Let W be the value of the [[Window]] internal slot of this.
// 2. Check if an access between two browsing contexts should be reported, given the current global object's browsing context, W's browsing context, P, and the current settings object.
check_if_access_between_two_browsing_contexts_should_be_reported(*verify_cast<Window>(current_global_object()).browsing_context(), *m_window->browsing_context(), property_key, current_settings_object());
check_if_access_between_two_browsing_contexts_should_be_reported(*verify_cast<Window>(current_global_object()).browsing_context(), m_window->browsing_context(), property_key, current_settings_object());
// 3. If IsPlatformObjectSameOrigin(W) is true, then return ? OrdinaryGet(this, P, Receiver).
// NOTE: this is passed rather than W as OrdinaryGet and CrossOriginGet will invoke the [[GetOwnProperty]] internal method.
@ -178,7 +178,7 @@ JS::ThrowCompletionOr<bool> WindowProxy::internal_set(JS::PropertyKey const& pro
// 1. Let W be the value of the [[Window]] internal slot of this.
// 2. Check if an access between two browsing contexts should be reported, given the current global object's browsing context, W's browsing context, P, and the current settings object.
check_if_access_between_two_browsing_contexts_should_be_reported(*verify_cast<Window>(current_global_object()).browsing_context(), *m_window->browsing_context(), property_key, current_settings_object());
check_if_access_between_two_browsing_contexts_should_be_reported(*verify_cast<Window>(current_global_object()).browsing_context(), m_window->browsing_context(), property_key, current_settings_object());
// 3. If IsPlatformObjectSameOrigin(W) is true, then:
if (is_platform_object_same_origin(*m_window)) {