Automatically applying components based on crash stacktrace and information from OWNERS files.

If this is incorrect, please apply the Test-Predator-Wrong-Components label.

Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/chromium/src/+/5361078a2944c3b70c5092bebecbf042fb824fc0 (Implement Element.toggleAttribute).

If this is incorrect, please let us know why and apply the Test-Predator-Wrong-CLs label. If you aren't the correct owner for this issue, please unassign yourself as soon as possible so it can be re-triaged.

yukishiino@, can you have a look at this? It shouldn't both be Pri-1 and unassigned. It looks like it might legitimately be a Pri-1 (CHECK failure on a security check), so assigning it to you rather than knocking down the priority.

The issue is reproducible with file:// protocol and not reproducible with http://.  It seems that, in the repro case, file://-1000000/ is treated as cross origin to file:///.  Hmm.

However, chrome allows some (or most?) of access from file:/// to file://something/ when --allow-file-access-from-files is specified.

The issue seems to me being caused by an inconsistency brought by --allow-file-access-from-files.  Some accesses are allowed while some others are not.

As this issue is not reproducible with http:// and needs a command-line flag, I think this is not security sensitive.

By the way, Firefox seems normalizing file://something/ into file:///.  At least the URL bar shows file:///.  Chrome shows file://something/ as is.

I've found the root cause that the implementation of SecurityOrigin (and LocalWindowProxy::SetSecurityToken) is inconsistent.

In case of file: scheme, SecurityOrigin::CanAccess requires that both of scheme (protocol) and host match.  However, SecurityOrigin::ToString (that is used as a security token) returns "file://" in case of file: scheme.  This is inconsistent.

If we care about host, then the security token must be "file://" + Host().

There are similar functions as SecurityOrigin::ToString, ToAtomicString, and ToRawString.  And they're used in many places.  I'm not sure if we need to fix all of them (will break several backward compatibilities), or it's okay to fix only the case of the security token (minimize the breakage of the backward compatibilities).

How should we proceed?  It's easy to only fix the security token with avoiding any other side effect, but not sure whether it's enough or not.  Can someone advise?


LocalWindowProxy::SetSecurityToken
https://cs.chromium.org/chromium/src/third_party/blink/renderer/bindings/core/v8/local_window_proxy.cc?l=383&rcl=8a49ff6a3074257bdf00a33468d909ce0007bbff

SecurityOrigin::ToString
https://cs.chromium.org/chromium/src/third_party/blink/renderer/platform/weborigin/security_origin.cc?l=403&rcl=7868f58d7acb0dcee73d335e6f28fb2d371dcdfe

SecurityOrigin::CanAccess
https://cs.chromium.org/chromium/src/third_party/blink/renderer/platform/weborigin/security_origin.cc?l=269&rcl=7868f58d7acb0dcee73d335e6f28fb2d371dcdfe

Daniel: Do you have any advice on this?

Made a (temporary?) fix at https://chromium-review.googlesource.com/c/chromium/src/+/1133105 .  IMHO, this is a reasonable fix for the time being until when we get a good understanding.

ClusterFuzz has detected this issue as fixed in range 574172:574173.

Detailed report: https://clusterfuzz.com/testcase?key=5887584711213056

Fuzzer: lcamtuf_cross_fuzz
Job Type: linux_ubsan_vptr_chrome
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  !scope.AccessCheckFailed() in v8_dom_wrapper.cc
  blink::V8DOMWrapper::CreateWrapper
  blink::ScriptWrappable::Wrap
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_ubsan_vptr_chrome&range=570347:570348
Fixed: https://clusterfuzz.com/revisions?job=linux_ubsan_vptr_chrome&range=574172:574173

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5887584711213056

See https://github.com/google/clusterfuzz-tools for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.

ClusterFuzz testcase 5887584711213056 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.

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

commit a9cd83acd7c28efa8ac1c79cd213f3138813ab83
Author: Yuki Shiino <yukishiino@chromium.org>
Date: Fri Jul 27 08:58:12 2018

v8binding: Make the security token consistent with access check.

SecurityOrigin::CanAccess distinguishes file://host/ from file:///
(empty host), however, the security tokens for file: scheme were
always "file://" regardless of host.  This makes Blink think two
origins are cross origin but V8 think the two are the same origin.

This patch changes the security tokens in case of file: scheme to
null string (will fallback to the default security token and V8
will always ask Blink to perform a security check).

Bug:  858990 
Change-Id: If305a994d6150984e8dd41b964003ce1eafb8b99
Reviewed-on: https://chromium-review.googlesource.com/1133105
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578568}
[modify] https://crrev.com/a9cd83acd7c28efa8ac1c79cd213f3138813ab83/third_party/blink/renderer/bindings/core/v8/local_window_proxy.cc
[modify] https://crrev.com/a9cd83acd7c28efa8ac1c79cd213f3138813ab83/third_party/blink/renderer/platform/weborigin/security_origin.cc
[modify] https://crrev.com/a9cd83acd7c28efa8ac1c79cd213f3138813ab83/third_party/blink/renderer/platform/weborigin/security_origin.h
[modify] https://crrev.com/a9cd83acd7c28efa8ac1c79cd213f3138813ab83/third_party/blink/renderer/platform/weborigin/security_origin_test.cc