New issue
Advanced search Search tips

Issue 841605 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Feature-policy: Introduce a feature to limit dynamic markup insertion API affecting document's stream

Project Member Reported by ekaramad@chromium.org, May 9 2018

Issue description

This effectively means document.{write, writeln, open, close}.

We should start by implementing the feature behind the experimental flag and eventually figure out the right and safe approach for cross and same origin frames.
 
Project Member

Comment 1 by bugdroid1@chromium.org, May 15 2018

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

commit 3f19764223c1e0646b393a5abcd37770b4ca8db0
Author: Ehsan Karamad <ekaramad@chromium.org>
Date: Tue May 15 21:51:10 2018

Introduce feature policy: document-stream-insertion

This CL introduces a new experimental feature
'document-stream-insertion' whose purpose is to limit the usage of
specific (anti-pattern) javascript API for "dynamic markup insertion".

The list of javascript methods to be blocked by this feature are:
document.{write, writeln, open, close}.
The set of disabled APIs is a subset of "dynamic-markup-insertion" from
HTML spec:

https://dev.w3.org/html5/spec-LC/apis-in-html-documents.html#dynamic-markup-insertion
https://html.spec.whatwg.org/multipage/dynamic-markup-insertion.html#dynamic-markup-insertion

Bug:  841605 
Change-Id: I9cc31fab36e2cea70cdce575e3868ce1d0cfecfa
Reviewed-on: https://chromium-review.googlesource.com/1053349
Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Ojan Vafai <ojan@chromium.org>
Reviewed-by: Ian Clelland <iclelland@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558847}
[modify] https://crrev.com/3f19764223c1e0646b393a5abcd37770b4ca8db0/content/browser/frame_host/render_frame_host_feature_policy_unittest.cc
[modify] https://crrev.com/3f19764223c1e0646b393a5abcd37770b4ca8db0/third_party/blink/common/feature_policy/feature_policy.cc
[modify] https://crrev.com/3f19764223c1e0646b393a5abcd37770b4ca8db0/third_party/blink/public/mojom/feature_policy/feature_policy.mojom
[modify] https://crrev.com/3f19764223c1e0646b393a5abcd37770b4ca8db0/third_party/blink/renderer/platform/feature_policy/feature_policy.cc

Project Member

Comment 2 by bugdroid1@chromium.org, May 23 2018

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

commit d8e49fe9a8374c236010d75874c082e9701543fb
Author: Ehsan Karamad <ekaramad@chromium.org>
Date: Wed May 23 22:17:58 2018

Implement policy: 'document-stream-insertion'

This CL adds the actual implementation for the experimental policy
'document-stream-insertion'. The policy is used to block usages of
specific APIs mentioned section "dynamic markup insertion" of the HTML
spec. This essentially includes document.{close, open, write, writeln}.

With the current CL, the calls to banned API lead to a DOMException.
The feature itself was introduced in a previous CL:
https://chromium-review.googlesource.com/c/chromium/src/+/1053349

Bug:  841605 
Change-Id: I1a764bc7545a0d26a29d217027cf43e561d8dfbd
Reviewed-on: https://chromium-review.googlesource.com/1058138
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Reviewed-by: Ian Clelland <iclelland@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561275}
[add] https://crrev.com/d8e49fe9a8374c236010d75874c082e9701543fb/third_party/WebKit/LayoutTests/external/wpt/feature-policy/experimental-features/document-stream-insertion.tentative.html
[add] https://crrev.com/d8e49fe9a8374c236010d75874c082e9701543fb/third_party/WebKit/LayoutTests/external/wpt/feature-policy/experimental-features/resources/common.js
[add] https://crrev.com/d8e49fe9a8374c236010d75874c082e9701543fb/third_party/WebKit/LayoutTests/external/wpt/feature-policy/experimental-features/resources/document-stream-insertion.html
[modify] https://crrev.com/d8e49fe9a8374c236010d75874c082e9701543fb/third_party/WebKit/LayoutTests/external/wpt/feature-policy/experimental-features/resources/vertical-scroll.js
[modify] https://crrev.com/d8e49fe9a8374c236010d75874c082e9701543fb/third_party/WebKit/LayoutTests/external/wpt/feature-policy/experimental-features/vertical-scroll-scrollintoview.tentative.html
[modify] https://crrev.com/d8e49fe9a8374c236010d75874c082e9701543fb/third_party/WebKit/LayoutTests/external/wpt/feature-policy/experimental-features/vertical-scroll-touch-action-manual.tentative.html
[modify] https://crrev.com/d8e49fe9a8374c236010d75874c082e9701543fb/third_party/blink/renderer/core/dom/document.cc
[modify] https://crrev.com/d8e49fe9a8374c236010d75874c082e9701543fb/third_party/blink/renderer/core/dom/document.h

Status: Fixed (was: Assigned)
Marking the bug as fixed following comment #2.
Project Member

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

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

commit 16f81534cc96de487f53312382996da7c6244972
Author: Ehsan Karamad <ekaramad@chromium.org>
Date: Wed Jun 13 14:39:40 2018

Rename Feature Policy: document-stream-insertion

It was proposed here https://github.com/WICG/feature-policy/pull/172
that "document-write" is a better name for this feature. This CL makes
the naming change correspondingly.

Bug:  841605 
Change-Id: Id0a928fd3f6669eb3700736ff5770d4717d2b414
Reviewed-on: https://chromium-review.googlesource.com/1095328
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Reviewed-by: Ian Clelland <iclelland@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566829}
[modify] https://crrev.com/16f81534cc96de487f53312382996da7c6244972/content/browser/frame_host/render_frame_host_feature_policy_unittest.cc
[rename] https://crrev.com/16f81534cc96de487f53312382996da7c6244972/third_party/WebKit/LayoutTests/external/wpt/feature-policy/experimental-features/document-write.tentative.html
[rename] https://crrev.com/16f81534cc96de487f53312382996da7c6244972/third_party/WebKit/LayoutTests/external/wpt/feature-policy/experimental-features/resources/document-write.html
[modify] https://crrev.com/16f81534cc96de487f53312382996da7c6244972/third_party/blink/common/feature_policy/feature_policy.cc
[modify] https://crrev.com/16f81534cc96de487f53312382996da7c6244972/third_party/blink/public/mojom/feature_policy/feature_policy.mojom
[modify] https://crrev.com/16f81534cc96de487f53312382996da7c6244972/third_party/blink/renderer/core/dom/document.cc
[modify] https://crrev.com/16f81534cc96de487f53312382996da7c6244972/third_party/blink/renderer/platform/feature_policy/feature_policy.cc

Sign in to add a comment