New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 787905 link

Starred by 3 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac , Fuchsia
Pri: 3
Type: Feature



Sign in to add a comment

Warn when JavaScript assigns to document.domain

Project Member Reported by palmer@chromium.org, Nov 22 2017

Issue description

"Domain relaxation" is a security anti-pattern:

```
// In a page at noodles.example.com:
document.domain = example.com
```

We should at least warn in the console when developers do this.
 

Comment 1 by tsepez@chromium.org, Nov 22 2017

Owner: tsepez@chromium.org
Status: Assigned (was: Available)
Cobbled together a quick CL to do this at
https://chromium-review.googlesource.com/c/chromium/src/+/786541

Comment 2 by mkwst@chromium.org, Nov 23 2017

I think we need a little more of a plan before we can add console messages, because this is used practically everywhere: https://www.chromestatus.com/metrics/feature/timeline/popularity/739 shows it as being around 11%, and adding a console message to one out of ten pages is pretty spammy.

As long as large third-parties like Facebook and Doubleclick are using this in their embedded code, it's not something we're going to be able to get away with removing from the platform. Likewise, until we've done some research into that ~11% to see whether the assignment has any effect, it's not clear that announcing a deprecation would be worthwhile. Walking back deprecation announcements hurts our credibility with developers, so I'd like things to be a little more planned out before moving forward.

One step I think we can take in the short term is to add metrics around origin comparisons that passed only because of relaxation. That is, out of that 11%, I'd like to understand how many pages would actually be affected if we removed the feature. I tried to do this a while ago, but we unfortunately don't pass anything useful into `SecurityOrigin::CanAccess`: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/weborigin/SecurityOrigin.cpp?l=261&gs=cpp%253Ablink%253A%253Aclass-SecurityOrigin%253A%253ACanAccess(const%2Bblink%253A%253ASecurityOrigin%2B*)-const%2540chromium%252F..%252F..%252Fthird_party%252FWebKit%252FSource%252Fplatform%252Fweborigin%252FSecurityOrigin.cpp%257Cdef&gsn=CanAccess&ct=xref_usages. So we'd either need to refactor the callsites, or refactor `SecurityOrigin` to hold a reference to a counter, or something else I haven't thought of yet.

Would you be interested in tackling that, Tom/Chris?

Comment 3 by tsepez@chromium.org, Nov 27 2017

Owner: ----
Status: Available (was: Assigned)
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 7

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

commit 41c5a44c00624453edc750e5bdd8dd2b44724f00
Author: Mike West <mkwst@chromium.org>
Date: Fri Sep 07 09:17:17 2018

Add a UseCounter for cross-origin accesses requiring 'document.domain'.

Bug: 787905
Change-Id: I2248173947ed87ff3e8f40dc86a8e9044ed2dd3b
Reviewed-on: https://chromium-review.googlesource.com/966442
Commit-Queue: Mike West <mkwst@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589470}
[modify] https://crrev.com/41c5a44c00624453edc750e5bdd8dd2b44724f00/third_party/blink/public/platform/web_feature.mojom
[modify] https://crrev.com/41c5a44c00624453edc750e5bdd8dd2b44724f00/third_party/blink/renderer/bindings/core/v8/binding_security.cc
[modify] https://crrev.com/41c5a44c00624453edc750e5bdd8dd2b44724f00/third_party/blink/renderer/bindings/core/v8/binding_security_test.cc
[modify] https://crrev.com/41c5a44c00624453edc750e5bdd8dd2b44724f00/third_party/blink/renderer/platform/weborigin/security_origin.cc
[modify] https://crrev.com/41c5a44c00624453edc750e5bdd8dd2b44724f00/third_party/blink/renderer/platform/weborigin/security_origin.h
[modify] https://crrev.com/41c5a44c00624453edc750e5bdd8dd2b44724f00/third_party/blink/renderer/platform/weborigin/security_origin_test.cc
[modify] https://crrev.com/41c5a44c00624453edc750e5bdd8dd2b44724f00/tools/metrics/histograms/enums.xml

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 7

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

commit 9164d759ad76787a21508b856ae8463fcf215357
Author: Markus Heintz <markusheintz@chromium.org>
Date: Fri Sep 07 15:34:29 2018

Revert "Add a UseCounter for cross-origin accesses requiring 'document.domain'."

This reverts commit 41c5a44c00624453edc750e5bdd8dd2b44724f00.

Reason for revert: This broke many webkit tests:

https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium.webkit%2FWebKit_Linux_Trusty_MSAN%2F9844%2F%2B%2Frecipes%2Fsteps%2Fwebkit_layout_tests%2F0%2Fstdout

In https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Linux%20Trusty%20MSAN/9844

Revert speculatively since the error message is pointing to code potentially added by this cl 

04:21:46.301 9979       #0 0x12242776 in blink::(anonymous namespace)::CanAccessWindowInternal(blink::LocalDOMWindow const*, blink::DOMWindow const*) ./../../third_party/blink/renderer/bindings/core/v8/binding_security.cc:73:79
04:21:46.301 9979       #1 0x1223dfac in CanAccessWindow ./../../third_party/blink/renderer/bindings/core/v8/binding_security.cc:110:7
04:21:46.301 9979       #2 0x1223dfac in blink::BindingSecurity::ShouldAllowAccessTo(blink::LocalDOMWindow const*, blink::DOMWindow const*, blink::BindingSecurity::ErrorReportOption) ./../../third_party/blink/renderer/bindings/core/v8/binding_se

Original change's description:
> Add a UseCounter for cross-origin accesses requiring 'document.domain'.
> 
> Bug: 787905
> Change-Id: I2248173947ed87ff3e8f40dc86a8e9044ed2dd3b
> Reviewed-on: https://chromium-review.googlesource.com/966442
> Commit-Queue: Mike West <mkwst@chromium.org>
> Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#589470}

TBR=dcheng@chromium.org,yukishiino@chromium.org,jochen@chromium.org,mkwst@chromium.org

Change-Id: Ib5c8fe269823c889c97da13e5107d1da75d7a457
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 787905
Reviewed-on: https://chromium-review.googlesource.com/1213152
Reviewed-by: Markus Heintz <markusheintz@chromium.org>
Commit-Queue: Markus Heintz <markusheintz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589524}
[modify] https://crrev.com/9164d759ad76787a21508b856ae8463fcf215357/third_party/blink/public/platform/web_feature.mojom
[modify] https://crrev.com/9164d759ad76787a21508b856ae8463fcf215357/third_party/blink/renderer/bindings/core/v8/binding_security.cc
[modify] https://crrev.com/9164d759ad76787a21508b856ae8463fcf215357/third_party/blink/renderer/bindings/core/v8/binding_security_test.cc
[modify] https://crrev.com/9164d759ad76787a21508b856ae8463fcf215357/third_party/blink/renderer/platform/weborigin/security_origin.cc
[modify] https://crrev.com/9164d759ad76787a21508b856ae8463fcf215357/third_party/blink/renderer/platform/weborigin/security_origin.h
[modify] https://crrev.com/9164d759ad76787a21508b856ae8463fcf215357/third_party/blink/renderer/platform/weborigin/security_origin_test.cc
[modify] https://crrev.com/9164d759ad76787a21508b856ae8463fcf215357/tools/metrics/histograms/enums.xml

Project Member

Comment 6 by bugdroid1@chromium.org, Sep 11

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

commit d8d471539b78aa71de46265e15f1cfef6ea1d7b4
Author: Mike West <mkwst@chromium.org>
Date: Tue Sep 11 06:53:37 2018

Add a UseCounter for cross-origin accesses requiring 'document.domain'.

This is a re-land of [1] after MSAN failures caused a rollback in [2].

[1]: https://chromium-review.googlesource.com/c/chromium/src/+/966442
[2]: https://chromium-review.googlesource.com/c/chromium/src/+/1213152

Bug: 787905
Change-Id: Ie4632a3a482c77202e7e4d3b5c360f021e628389
Reviewed-on: https://chromium-review.googlesource.com/1216684
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Andy Paicu <andypaicu@chromium.org>
Commit-Queue: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590219}
[modify] https://crrev.com/d8d471539b78aa71de46265e15f1cfef6ea1d7b4/third_party/blink/public/platform/web_feature.mojom
[modify] https://crrev.com/d8d471539b78aa71de46265e15f1cfef6ea1d7b4/third_party/blink/renderer/bindings/core/v8/binding_security.cc
[modify] https://crrev.com/d8d471539b78aa71de46265e15f1cfef6ea1d7b4/third_party/blink/renderer/bindings/core/v8/binding_security_test.cc
[modify] https://crrev.com/d8d471539b78aa71de46265e15f1cfef6ea1d7b4/third_party/blink/renderer/platform/weborigin/security_origin.cc
[modify] https://crrev.com/d8d471539b78aa71de46265e15f1cfef6ea1d7b4/third_party/blink/renderer/platform/weborigin/security_origin.h
[modify] https://crrev.com/d8d471539b78aa71de46265e15f1cfef6ea1d7b4/third_party/blink/renderer/platform/weborigin/security_origin_test.cc
[modify] https://crrev.com/d8d471539b78aa71de46265e15f1cfef6ea1d7b4/tools/metrics/histograms/enums.xml

Project Member

Comment 7 by bugdroid1@chromium.org, Sep 14

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

commit 0989a6747d94349942269f803e57d3db0b25e75d
Author: Mike West <mkwst@chromium.org>
Date: Fri Sep 14 08:28:59 2018

Add `document.domain` use counters to UKM.

Bug: 787905
Change-Id: Icbf2459fbd8bd60f6fc016624f72ec704491897b
Reviewed-on: https://chromium-review.googlesource.com/1223368
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591302}
[modify] https://crrev.com/0989a6747d94349942269f803e57d3db0b25e75d/chrome/browser/page_load_metrics/observers/use_counter/ukm_features.cc

Sign in to add a comment