Commit 5f549907 authored by Joey Arhar's avatar Joey Arhar Committed by Commit Bot

Fix racy origin_document->GetFrame() check

This bug was caused by crrev.com/c/2212458 because it allows async form
navigations to outlive the document which initiated them, because it is
stored on the target frame instead of the initiating document. This
means that crrev.com/c/2212458 did technically change this behavior,
however, I don't think this is an issue because:
1. It is very racy to observe. The clusterfuzz test case appears to
   repeatedly reload the page very fast, and it takes several loads
   before the state where the async form task outliving the initiating
   document occurs.
2. This matches the behavior we used to have at the beginning of 2019
   before sync form submissions, which means that it wasn't a problem
   for websites a year ago.

The reason this null object deference was hit is because it was added
after sync form submissions. If the sync form submission change never
happened, then this same clusterfuzz bug would have been raised by
http://crrev.com/c/2090095
You can see other uses of origin_document->GetFrame() in the same
function check if GetFrame() is null before using it, probably because
they were written before the sync form submission patch.

Due to the racy nature of this bug, I couldn't think of a test case
which would reliably reproduce it.

Bug: 1088006
Change-Id: Iec2b182196a1d3adde57c23c65195def5d37a39a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2223009Reviewed-by: default avatarMason Freed <masonfreed@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#773420}
parent d7f46e2f
......@@ -557,9 +557,12 @@ void LocalFrameClientImpl::BeginNavigation(
should_check_main_world_content_security_policy;
navigation_info->blob_url_token = blob_url_token.PassPipe();
navigation_info->input_start = input_start_time;
navigation_info->initiator_frame =
origin_document ? origin_document->GetFrame()->Client()->GetWebFrame()
: nullptr;
if (origin_document && origin_document->GetFrame()) {
navigation_info->initiator_frame =
origin_document->GetFrame()->Client()->GetWebFrame();
} else {
navigation_info->initiator_frame = nullptr;
}
for (auto& policy : initiator_csp) {
navigation_info->initiator_csp.emplace_back(
ConvertToPublic(std::move(policy)));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment