New issue
Advanced search Search tips

Issue 705658 link

Starred by 1 user
Status: Fixed
Owner:
iclell...@chromium.org
Closed: May 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug

Blocking:
issue 623682



Sign in to add a comment

Feature policy allow attribute causes crash with --site-per-process

Project Member Reported by iclell...@chromium.org, Mar 27 2017 
If a pages includes an iframe with an allow attribute, the renderer will crash, if running in site-isolation mode.

The vector equality check in content/common/frame_owner_properties.cc doesn't take into account the fact that other.allowed_features may have fewer elements than this->allowed_features, and can run past the end of the vector during the compare.

Comment 1 by iclell...@chromium.org, Mar 31 2017 

Note that this crash did not surface in SitePerProcessBrowsertests, as the iframes in the test data file had a name attribute, in addition to the allow attributes. This meant that the FrameOwnerProperties::operator== comparison terminated early, and never tested the case where the two FOP objects were identical except for the allow attributes.

The test file has been updated so that this condition is actually tested, with a link back to this bug for reference. I'll also add unit tests for the operator== method for completeness.
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 31 2017 

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

commit aee9dcacf21624198c60857788c35906fffa97ad
Author: iclelland <iclelland@chromium.org>
Date: Fri Mar 31 14:08:09 2017

Fix crash in frame owner property replication.

Also updated the test that can trigger it, removing the unnecessary name
attributes that were masking the error.

BUG= 705658 

Review-Url: https://codereview.chromium.org/2775393003
Cr-Commit-Position: refs/heads/master@{#461111}

[modify] https://crrev.com/aee9dcacf21624198c60857788c35906fffa97ad/content/common/frame_owner_properties.cc
[modify] https://crrev.com/aee9dcacf21624198c60857788c35906fffa97ad/content/test/data/allowed_frames.html

Comment 3 by iclell...@chromium.org, May 3 2017

Status: Fixed (was: Started)
►

Sign in to add a comment