New issue
Advanced search Search tips

Issue 849311 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

Pending frame policy shouldn't be committed by a same-document navigation.

Project Member Reported by alex...@chromium.org, Jun 4 2018

Issue description

Splitting this off from  https://crbug.com/825677#c15 .  It looks like RenderFrameHostManager::DidNavigateFrame() is calling CommitPendingFramePolicy() even for same-document navigations, which is incorrect: those navigations shouldn't cause updated sandbox flags or feature policy to take effect.   In practice this means that a same-document commit would send the committed frame policy to proxies, and if a proxy were to later add a subframe, it might end up with incorrect sandbox flags or feature policy.

I'll put together a fix, which should be straightforward.

 
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 4 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ade42bd256628b51f9969b798339ce9bfbc9ae01

commit ade42bd256628b51f9969b798339ce9bfbc9ae01
Author: Alex Moshchuk <alexmos@chromium.org>
Date: Mon Jun 04 23:37:18 2018

Do not commit pending frame policy on same-document navigations.

When a frame updates a child frame's sandbox flags or feature policy,
the update should only take effect when the child commits a
cross-document navigation.  Same-document navigations, such as
navigating to a hash or pushState/replaceState, should not trigger the
update.

Bug:  849311 
Change-Id: Ie99de2225c790f86818dfb5c5aed13d0aad6427d
Reviewed-on: https://chromium-review.googlesource.com/1085627
Reviewed-by: Ian Clelland <iclelland@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564294}
[modify] https://crrev.com/ade42bd256628b51f9969b798339ce9bfbc9ae01/content/browser/frame_host/render_frame_host_manager.cc
[modify] https://crrev.com/ade42bd256628b51f9969b798339ce9bfbc9ae01/content/browser/site_per_process_browsertest.cc

Labels: Merge-Request-68 OS-Chrome OS-Linux OS-Mac OS-Windows
The fix is very simple and I think for correctness it would be good to have in M68, so requesting merge.
Labels: -Merge-Request-68 Merge-Approved-68
Approving merge for M68. Branch:3440
Project Member

Comment 4 by bugdroid1@chromium.org, Jun 8 2018

Labels: -merge-approved-68 merge-merged-3440
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f01342b58f0827e5de61a7841a6b961abbed2ae7

commit f01342b58f0827e5de61a7841a6b961abbed2ae7
Author: Alex Moshchuk <alexmos@chromium.org>
Date: Fri Jun 08 04:27:17 2018

Do not commit pending frame policy on same-document navigations. (Merge to M68)

When a frame updates a child frame's sandbox flags or feature policy,
the update should only take effect when the child commits a
cross-document navigation.  Same-document navigations, such as
navigating to a hash or pushState/replaceState, should not trigger the
update.

TBR=alexmos@chromium.org

(cherry picked from commit ade42bd256628b51f9969b798339ce9bfbc9ae01)

Bug:  849311 
Change-Id: Ie99de2225c790f86818dfb5c5aed13d0aad6427d
Reviewed-on: https://chromium-review.googlesource.com/1085627
Reviewed-by: Ian Clelland <iclelland@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#564294}
Reviewed-on: https://chromium-review.googlesource.com/1092310
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#255}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[modify] https://crrev.com/f01342b58f0827e5de61a7841a6b961abbed2ae7/content/browser/frame_host/render_frame_host_manager.cc
[modify] https://crrev.com/f01342b58f0827e5de61a7841a6b961abbed2ae7/content/browser/site_per_process_browsertest.cc

Status: Fixed (was: Started)

Sign in to add a comment