!source_url.is_empty() in render_frame_impl.cc |
||||||||
Issue descriptionDetailed report: https://cluster-fuzz.appspot.com/testcase?key=6404769052884992 Fuzzer: inferno_layout_test_unmodified Job Type: linux_debug_content_shell_drt Platform Id: linux Crash Type: CHECK failure Crash Address: Crash State: !source_url.is_empty() in render_frame_impl.cc content::RenderFrameImpl::decidePolicyForNavigation blink::FrameLoaderClientImpl::decidePolicyForNavigation Minimized Testcase (0.16 Kb): Download: https://cluster-fuzz.appspot.com/download/AMIfv97ij_XuPApJ-FvU0LRns4vwjYeV1K2uJ8hPCWVKknhxrjiCzIUUJGmMiXaem1Vgasw1v7G0ZvzeDZEtgo_XzYARSvQcGg3NYNU4-BJcpSVSulbXYILitIb8mbAK1nw6jTo7C6CjUlQiO4gtBsFUW8-q-7Nz9g?testcase_id=6404769052884992 <script> var __v_212 = window.open("resources/reentrant-beforeunload-helper.html"); __v_212.open( "resources/xmlhttprequest-no-content-type-with-text.php"); </script> Filer: mummareddy See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
,
Jun 22 2016
Looks like it's failing this DCHECK?
if (!should_fork && url.SchemeIs(url::kFileScheme)) {
// Fork non-file to file opens. Check the opener URL if this is the
// initial navigation in a newly opened window.
GURL source_url(old_url);
if (is_initial_navigation && source_url.is_empty() && frame_->opener())
source_url = frame_->opener()->top()->document().url();
DCHECK(!source_url.is_empty());
should_fork = !source_url.SchemeIs(url::kFileScheme);
}
tsepez@ added that in https://codereview.chromium.org/10517009. Tom, can you take a look?
Looks like just a DCHECK failure, but that code does look like it has a bug. The source URL could still be empty if the window had a chain of multiple openers or if the opener were cleared. Matters in the case that the navigation happens before the first commit, and it might bypass the intent of the security check in release builds (when DCHECKs aren't present).
,
Jun 23 2016
Moving this nonessential bug to the next milestone. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 3 2016
This issue is Pri-1 but has already been moved once. Lowering the priority and moving to the next milestone. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 14 2016
Should be OK to just remove the DCHECK(), then.
,
Jul 14 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6573f1eaf729320beeaf776674b124bfe5c330bb commit 6573f1eaf729320beeaf776674b124bfe5c330bb Author: tsepez <tsepez@chromium.org> Date: Thu Jul 14 22:35:57 2016 Simplify RenderFrameImpl::decidePolicyForNavigation and avoid DCHECK. DCHECK can be tripped when there is a chain of openers or when the opener has been cleared. The subsequent comparison against file: fails, and we may make a browser navigation where one could have been avoided, but it is still correct. Since it is still correct, we can simplify the check, perhaps forking in more cases that we used to, but that doesn't matter. BUG= 622509 R=creis@chromium.org Review-Url: https://codereview.chromium.org/2153573002 Cr-Commit-Position: refs/heads/master@{#405609} [modify] https://crrev.com/6573f1eaf729320beeaf776674b124bfe5c330bb/content/renderer/render_frame_impl.cc
,
Jul 15 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/11f3ed430923329885682d50057217041ce661e1 commit 11f3ed430923329885682d50057217041ce661e1 Author: tkent <tkent@chromium.org> Date: Fri Jul 15 04:16:37 2016 Revert of Simplify RenderFrameImpl::decidePolicyForNavigation and avoid DCHECK (patchset #2 id:20001 of https://codereview.chromium.org/2153573002/ ) Reason for revert: Broke a layout test fast/events/popup-allowed-from-gesture-initiated-form-submit.html . http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&showLargeExpectations=true&tests=popup-allowed-from-gesture-initiated-form-submit.html Original issue's description: > Simplify RenderFrameImpl::decidePolicyForNavigation and avoid DCHECK. > > DCHECK can be tripped when there is a chain of openers or when the > opener has been cleared. The subsequent comparison against file: > fails, and we may make a browser navigation where one could have > been avoided, but it is still correct. > > Since it is still correct, we can simplify the check, perhaps > forking in more cases that we used to, but that doesn't matter. > > BUG= 622509 > R=creis@chromium.org > > Committed: https://crrev.com/6573f1eaf729320beeaf776674b124bfe5c330bb > Cr-Commit-Position: refs/heads/master@{#405609} TBR=creis@chromium.org,tsepez@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 622509 , 628478 Review-Url: https://codereview.chromium.org/2150243002 Cr-Commit-Position: refs/heads/master@{#405694} [modify] https://crrev.com/11f3ed430923329885682d50057217041ce661e1/content/renderer/render_frame_impl.cc
,
Jul 15 2016
ClusterFuzz has detected this issue as fixed in range 405563:405613. Detailed report: https://cluster-fuzz.appspot.com/testcase?key=6404769052884992 Fuzzer: inferno_layout_test_unmodified Job Type: linux_debug_content_shell_drt Platform Id: linux Crash Type: CHECK failure Crash Address: Crash State: !source_url.is_empty() in render_frame_impl.cc content::RenderFrameImpl::decidePolicyForNavigation blink::FrameLoaderClientImpl::decidePolicyForNavigation Fixed: https://cluster-fuzz.appspot.com/revisions?job=linux_debug_content_shell_drt&range=405563:405613 Minimized Testcase (0.16 Kb): Download: https://cluster-fuzz.appspot.com/download/AMIfv97ij_XuPApJ-FvU0LRns4vwjYeV1K2uJ8hPCWVKknhxrjiCzIUUJGmMiXaem1Vgasw1v7G0ZvzeDZEtgo_XzYARSvQcGg3NYNU4-BJcpSVSulbXYILitIb8mbAK1nw6jTo7C6CjUlQiO4gtBsFUW8-q-7Nz9g?testcase_id=6404769052884992 <script> var __v_212 = window.open("resources/reentrant-beforeunload-helper.html"); __v_212.open( "resources/xmlhttprequest-no-content-type-with-text.php"); </script> See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information. If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
,
Jul 15 2016
ClusterFuzz testcase is verified as fixed, closing issue. If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
,
Jul 15 2016
Reverted, bug reopened
,
Jul 15 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/77b02a9e18d0a359c570e871d970fc515d54a9aa commit 77b02a9e18d0a359c570e871d970fc515d54a9aa Author: tsepez <tsepez@chromium.org> Date: Fri Jul 15 22:21:11 2016 Remove DCHECK(!source_url.is_empty()) from RenderFrameImpl::decidePolicyForNavigation BUG= 622509 Review-Url: https://codereview.chromium.org/2152353002 Cr-Commit-Position: refs/heads/master@{#405877} [modify] https://crrev.com/77b02a9e18d0a359c570e871d970fc515d54a9aa/content/renderer/render_frame_impl.cc
,
Jul 15 2016
,
Nov 22 2016
Removing EditIssue view restrictions from ClusterFuzz filed bugs. If you believe that this issue should still be restricted, please reapply the label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by mummare...@chromium.org
, Jun 22 2016Labels: Te-Logged M-52
Owner: creis@chromium.org
Status: Assigned (was: Available)