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

Issue 894426 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Out until 24 Jan
Closed: Oct 16
Cc:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Mac
Pri: 1
Type: Bug



Sign in to add a comment

CHECK failure: !result.opaque() in security_origin.cc

Project Member Reported by ClusterFuzz, Oct 11

Issue description

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

Fuzzer: attekett_surku_fuzzer
Job Type: linux_msan_chrome
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  !result.opaque() in security_origin.cc
  blink::SecurityOrigin::ToUrlOrigin
  blink::SecurityPolicy::IsOriginAccessAllowed
  
Sanitizer: memory (MSAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_msan_chrome&range=598356:598373

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

Issue filed automatically.

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

Comment 1 by ClusterFuzz, Oct 12

Labels: OS-Android OS-Mac
Cc: kkaluri@chromium.org
Labels: M-71 Test-Predator-Wrong
Owner: nasko@chromium.org
Status: Assigned (was: Untriaged)
Predator could not provide any possible suspects.

Using CL for the file, "security_origin.cc" suspecting the below Cl might have caused this issue

Suspect CL: https://chromium.googlesource.com/chromium/src/+/99445acdfcc014c69c3df7e19ecf7242a536bcb7

nasko@ -- Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner.

Thanks!
Yes, this is caused by my change. I'll look into it.
Sadness ... there is a mismatch in behavior between KURL and GURL when it comes to default port for the gopher: scheme. It is trivially fixed by adding the default port to known_ports.cc, but also seems like the wrong thing to do. Since the original CL is in M71, I will put up a fix using that approach, so it can be merged back.

In parallel, I will poke at what it takes to remove support gopher: from the URL parsers in the codebase.
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 16

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

commit 116094fb693556d5a8aff336aa6d635490546fcf
Author: Nasko Oskov <nasko@chromium.org>
Date: Tue Oct 16 01:25:33 2018

Add default port for the gopher: scheme.

KURL parsing uses known_ports.cc for supporting known ports for standard
schemes. GURL uses its own version, which leads to mismatch in behavior.

This CL adds gopher's default port to known_ports.cc, however a follow
up patch should just remove gopher support in general.

Bug:  894426 
Change-Id: I9cde04bef34be39337cdb9f2a584de576d0b15ea
Reviewed-on: https://chromium-review.googlesource.com/c/1279408
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599812}
[modify] https://crrev.com/116094fb693556d5a8aff336aa6d635490546fcf/third_party/blink/renderer/platform/weborigin/known_ports.cc
[modify] https://crrev.com/116094fb693556d5a8aff336aa6d635490546fcf/third_party/blink/renderer/platform/weborigin/security_origin_test.cc
[modify] https://crrev.com/116094fb693556d5a8aff336aa6d635490546fcf/url/origin_unittest.cc

Project Member

Comment 6 by ClusterFuzz, Oct 16

ClusterFuzz has detected this issue as fixed in range 599811:599813.

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

Fuzzer: attekett_surku_fuzzer
Job Type: linux_msan_chrome
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  !result.opaque() in security_origin.cc
  blink::SecurityOrigin::ToUrlOrigin
  blink::SecurityPolicy::IsOriginAccessAllowed
  
Sanitizer: memory (MSAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_msan_chrome&range=598356:598373
Fixed: https://clusterfuzz.com/revisions?job=linux_msan_chrome&range=599811:599813

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

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.
Project Member

Comment 7 by ClusterFuzz, Oct 16

Labels: ClusterFuzz-Verified
Status: Verified (was: Assigned)
ClusterFuzz testcase 5263033973342208 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Labels: Merge-Request-71
Adding merge request for M71, since the original patch causing the issue landed late in the M71 cycle. The fix is very safe.
Labels: -Merge-Request-71 Merge-Approved-71
Approving merge to M71 branch 3578 based on comment #8.
 Pls merge your change to M71 branch #3578 ASAP so we can pick it up for next M71 Beta release. Thank you.

Project Member

Comment 10 by bugdroid1@chromium.org, Oct 19

Labels: -merge-approved-71 merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/52e79d0d979d0fa365f7c0c53e4e3a71199a623e

commit 52e79d0d979d0fa365f7c0c53e4e3a71199a623e
Author: Nasko Oskov <nasko@chromium.org>
Date: Fri Oct 19 22:09:14 2018

Add default port for the gopher: scheme.

KURL parsing uses known_ports.cc for supporting known ports for standard
schemes. GURL uses its own version, which leads to mismatch in behavior.

This CL adds gopher's default port to known_ports.cc, however a follow
up patch should just remove gopher support in general.

Bug:  894426 
Change-Id: I9cde04bef34be39337cdb9f2a584de576d0b15ea
Reviewed-on: https://chromium-review.googlesource.com/c/1279408
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Nasko Oskov <nasko@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#599812}(cherry picked from commit 116094fb693556d5a8aff336aa6d635490546fcf)
Reviewed-on: https://chromium-review.googlesource.com/c/1292715
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#173}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/52e79d0d979d0fa365f7c0c53e4e3a71199a623e/third_party/blink/renderer/platform/weborigin/known_ports.cc
[modify] https://crrev.com/52e79d0d979d0fa365f7c0c53e4e3a71199a623e/third_party/blink/renderer/platform/weborigin/security_origin_test.cc
[modify] https://crrev.com/52e79d0d979d0fa365f7c0c53e4e3a71199a623e/url/origin_unittest.cc

Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/52e79d0d979d0fa365f7c0c53e4e3a71199a623e

Commit: 52e79d0d979d0fa365f7c0c53e4e3a71199a623e
Author: nasko@chromium.org
Commiter: nasko@chromium.org
Date: 2018-10-19 22:09:14 +0000 UTC

Add default port for the gopher: scheme.

KURL parsing uses known_ports.cc for supporting known ports for standard
schemes. GURL uses its own version, which leads to mismatch in behavior.

This CL adds gopher's default port to known_ports.cc, however a follow
up patch should just remove gopher support in general.

Bug:  894426 
Change-Id: I9cde04bef34be39337cdb9f2a584de576d0b15ea
Reviewed-on: https://chromium-review.googlesource.com/c/1279408
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Nasko Oskov <nasko@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#599812}(cherry picked from commit 116094fb693556d5a8aff336aa6d635490546fcf)
Reviewed-on: https://chromium-review.googlesource.com/c/1292715
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#173}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}

Sign in to add a comment