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

Issue 802259 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Mac
Pri: 1
Type: Bug



Sign in to add a comment

Timeout in feature_policy_fuzzer

Project Member Reported by ClusterFuzz, Jan 16 2018

Issue description

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

Fuzzer: libFuzzer_feature_policy_fuzzer
Job Type: libfuzzer_chrome_msan
Platform Id: linux

Crash Type: Timeout (exceeds 25 secs)
Crash Address: 
Crash State:
  feature_policy_fuzzer
  
Sanitizer: memory (MSAN)

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

Issue filed automatically.

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reference.md for more information.

Note: This crash might not be reproducible with the provided testcase. That said, for the past 14 days we've been seeing this crash frequently. If you are unable to reproduce this, please try a speculative fix based on the crash stacktrace in the report. The fix can be verified by looking at the crash statistics in the report, a day after the fix is deployed. We will auto-close the bug if the crash is not seen for 14 days.
 
Project Member

Comment 1 by ClusterFuzz, Jan 17 2018

Labels: OS-Mac
Cc: brajkumar@chromium.org
Components: Infra
Labels: Test-Predator-Wrong CF-NeedsTriage
Unable to find actual suspect through code search and also no regressed revision range is seen in the detailed report, hence adding appropriate label and marking it as untriaged.

Thanks!
Components: -Infra Tools>Stability>ClusterFuzz
Components: -Tools>Stability>ClusterFuzz Blink>FeaturePolicy
Owner: iclell...@chromium.org
Status: Assigned (was: Untriaged)
Braj, please do proper triage. You can always git blame on the fuzzer author feature_policy_fuzzer.
See 
https://chromium.googlesource.com/chromium/src/+/f9e719920abbe22174c11b007e660a8f11622018
The test case contains a string with an origin in it which is > 64kb. That string takes over 8 seconds to convert to a GURL on my Z620 workstation. (This is in the GURL constructor at https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/feature_policy/FeaturePolicy.cpp?sq=package:chromium&dr&l=184)

I'm not sure if this represents an actual regression anywhere, or if it's just an interesting (and very CPU-intensive) test case just now caught by the fuzzer.

This seems large, but is well within Chrome's 256KB header limit, so I'll take a look to see whether there's anything I can do to defend against this in the FeaturePolicy.cpp parser.
This comes down to a call to uidna_nameToASCII in url_canon_icu.cc with a 65577 element char16 array (131154 bytes) which, on my machine, takes between 6500 and 8200ms to return.

This code path is only hit when there are non-ASCII characters in the original input (otherwise the simple host canonicalization is used, which is much faster). Since those characters shouldn't appear in a serialized feature policy representation, I'll add some code to fail fast if they are found in the header.

Project Member

Comment 7 by bugdroid1@chromium.org, Jan 23 2018

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

commit f7031d799b80e96a11fdbc3012e96f1c484055a5
Author: Ian Clelland <iclelland@google.com>
Date: Tue Jan 23 13:50:27 2018

Ignore origins with non-ascii characters in Feature Policy.

Origins should be encoded into ASCII representations for use in feature
policy headers and attributes. This change makes the parser fail fast
when encoutering non-ASCII characters, rather than sending the origin
through the potentially slow ICU path before rejecting it.

Bug:  802259 
Change-Id: I3c3e26f06e9737aee850de0ca9ac92a4879c4fe6
Reviewed-on: https://chromium-review.googlesource.com/875919
Reviewed-by: Luna Lu <loonybear@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Ian Clelland <iclelland@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531220}
[modify] https://crrev.com/f7031d799b80e96a11fdbc3012e96f1c484055a5/third_party/WebKit/Source/platform/feature_policy/FeaturePolicy.cpp
[modify] https://crrev.com/f7031d799b80e96a11fdbc3012e96f1c484055a5/third_party/WebKit/Source/platform/feature_policy/FeaturePolicyTest.cpp

Status: Fixed (was: Assigned)

Sign in to add a comment