Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(111)

Issue 2497133002: Add a SECURITY_CHECK that HTMLFrameElementBase is a consistent state.

Created:
2 years, 10 months ago by dcheng
Modified:
2 years, 10 months ago
Reviewers:
esprehn
CC:
chromium-reviews, blink-reviews, dglazkov+blink, blink-reviews-html_chromium.org, jochen (gone - plz use gerrit), dominicc (has gone to gerrit)
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

SECURITY_CHECK that inserted HTMLFrameElementBases aren't already attached. The consequences of getting this wrong are quite bad. Corrupting Blink's state about the DOM tree and causing an already-attached frame to be reattached is a common UXSS primitive: if the frame is reattached with a javascript: URL, the normal security checks are bypassed, allowing cross-origin script execution. Taken from https://codereview.chromium.org/2169453002. BUG=663476

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+5 lines, -0 lines) Patch
M third_party/WebKit/Source/core/html/HTMLFrameElementBase.cpp View 2 chunks +5 lines, -0 lines 0 comments Download

Messages

Total messages: 15 (7 generated)
dcheng
This will cause UXSSes to crash the renderer instead, but I think that's preferable. We ...
2 years, 10 months ago (2016-11-12 04:14:28 UTC) #4
Mariusz Mlynski
It can't be guaranteed that there's no content frame, here's a simple example that will ...
2 years, 10 months ago (2016-11-12 22:24:54 UTC) #9
dcheng
On 2016/11/12 22:24:54, Mariusz Mlynski wrote: > It can't be guaranteed that there's no content ...
2 years, 10 months ago (2016-11-12 23:56:38 UTC) #10
dcheng
ping =)
2 years, 10 months ago (2016-11-15 02:46:39 UTC) #11
esprehn
On 2016/11/12 at 23:56:38, dcheng wrote: > On 2016/11/12 22:24:54, Mariusz Mlynski wrote: > > ...
2 years, 10 months ago (2016-11-15 03:36:03 UTC) #12
Mariusz Mlynski
On 2016/11/12 23:56:38, dcheng wrote: > I agree that that's not true in Blink today, ...
2 years, 10 months ago (2016-11-15 05:00:53 UTC) #13
dcheng
On 2016/11/15 05:00:53, Mariusz Mlynski wrote: > On 2016/11/12 23:56:38, dcheng wrote: > > I ...
2 years, 10 months ago (2016-11-15 05:11:16 UTC) #14
Mariusz Mlynski
2 years, 10 months ago (2016-11-15 07:30:40 UTC) #15
On 2016/11/15 05:11:16, dcheng wrote:
> Right, I don't think this is the fix for the bug in question, but this is far
> too commonly abused to turn any DOM corruption into a UXSS.

No, I mean DOM corruption UXSS'es in general, not 663476 specifically. I think
the root problem is that the security check is skipped in the first place.
Please see https://codereview.chromium.org/2502783004

Powered by Google App Engine
This is Rietveld 408576698