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

Issue 901675 link

Starred by 3 users

Issue metadata

Status: Verified
Owner:
hobby only
Closed: Nov 21
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug



Sign in to add a comment

Timeout in autofill_phone_number_i18n_fuzzer

Project Member Reported by ClusterFuzz, Nov 4

Issue description

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

Fuzzer: libFuzzer_autofill_phone_number_i18n_fuzzer
Job Type: libfuzzer_chrome_msan
Platform Id: linux

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

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_msan&range=600499:600510

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

Issue filed automatically.

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

Comment 1 by ClusterFuzz, Nov 4

Cc: vabr@chromium.org
Labels: ClusterFuzz-Auto-CC
Automatically adding ccs based on OWNERS file / target commit history.

If this is incorrect, please add ClusterFuzz-Wrong label.
Cc: mpdenton@chromium.org
Cc: kkaluri@chromium.org
Labels: M-72 Test-Predator-Wrong CF-NeedsTriage
Unable to find actual suspect through code search and also observing no CL's under regression range, hence adding appropriate label and requesting someone from dev team to look in to this issue.

Thanks!
Hmm, on my machine this particular run takes between 25 and 30 seconds (tested both with msan on and off). So while it would time out under 25 seconds limit, this is not the process being stuck. The stack trace from the report shows icu_63::RegexMatcher -- I guess parsing with regular expressions just takes a long time?

$ out/msan-libfuzzer/autofill_phone_number_i18n_fuzzer ~/Downloads/clusterfuzz-testcase-minimized-autofill_phone_number_i18n_fuzzer-5404640882196480 
INFO: Seed: 1143369890
INFO: Loaded 1 modules   (201956 inline 8-bit counters): 201956 [0x5619a6d91810, 0x5619a6dc2cf4), 
INFO: Loaded 1 PC tables (201956 PCs): 201956 [0x5619a6dc2cf8,0x5619a70d7b38), 
out/msan-libfuzzer/autofill_phone_number_i18n_fuzzer: Running 1 inputs 1 time(s) each.
Running: /usr/local/google/home/vabr/Downloads/clusterfuzz-testcase-minimized-autofill_phone_number_i18n_fuzzer-5404640882196480
UTF-8 buffer is not interchange-valid.Executed /usr/local/google/home/vabr/Downloads/clusterfuzz-testcase-minimized-autofill_phone_number_i18n_fuzzer-5404640882196480 in 28847 ms
***
*** NOTE: fuzzing was not performed, you have only
***       executed the target code on a fixed set of inputs.
***

$ out/libfuzzer/autofill_phone_number_i18n_fuzzer ~/Downloads/clusterfuzz-testcase-minimized-autofill_phone_number_i18n_fuzzer-5404640882196480 
INFO: Seed: 1213940288
INFO: Loaded 1 modules   (906809 inline 8-bit counters): 906809 [0x55e67f0eaf70, 0x55e67f1c85a9), 
INFO: Loaded 1 PC tables (906809 PCs): 906809 [0x55e67f1c85b0,0x55e67ff9e940), 
out/libfuzzer/autofill_phone_number_i18n_fuzzer: Running 1 inputs 1 time(s) each.
Running: /usr/local/google/home/vabr/Downloads/clusterfuzz-testcase-minimized-autofill_phone_number_i18n_fuzzer-5404640882196480
UTF-8 buffer is not interchange-valid.UTF-8 buffer is not interchange-valid.Executed /usr/local/google/home/vabr/Downloads/clusterfuzz-testcase-minimized-autofill_phone_number_i18n_fuzzer-5404640882196480 in 27097 ms
***
*** NOTE: fuzzing was not performed, you have only
***       executed the target code on a fixed set of inputs.
***

Components: UI>Browser>Passwords UI>Browser>Autofill
Labels: -CF-NeedsTriage
Cc: mmoroz@chromium.org
Labels: Needs-Feedback
Hi Max,

Would you know what is the preferred path forward here?

The hypothesis is that the fuzzed code takes slightly longer than the fuzzer timeout. I am not sure the autofill team is able to invest into improving the fuzzed code's performance unless there is a visible user benefit to it. Do we:
 (A) increase the timeout for this specific code, or
 (B) decrease the max fuzzer input length (the minimised testcase is 24KB) and perhaps put a hard max-length bound in the fuzzed code, or
 (C) let it be as it is, or
 (D) disable the fuzzer, or
 (E) something else?

My guess would be (B), because that limits the fuzzing resources and prevents exploits in the out-of-fuzzing range, but happy to hear your advice.

Thanks!
Vaclav
Labels: -Needs-Feedback
Owner: vabr@chromium.org
Status: Started (was: Untriaged)
Actually, for (B), there is no need to change the fuzzer params. If the code trims the excessive input, it will run fast enough. I think I might just try that. Still, please shout if you have better options.
Implemented in https://crrev.com/c/1343007
Vaclav, that solution SGTM! Indeed, there is no reason to expect insanely long phone numbers. Thanks for the fix, and please fill out https://docs.google.com/forms/d/e/1FAIpQLSePwWA3J2P0yXggjuD8jGSY5eQZY7eSRF43aZvA3IlcNkrGiQ/viewform (it's super quick) :)
Project Member

Comment 10 by ClusterFuzz, Nov 21

Labels: OS-Windows
Project Member

Comment 11 by bugdroid1@chromium.org, Nov 21

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

commit f2b4cc12fb1ef8ae8fdbbc8d138682c9c12cd466
Author: Vaclav Brozek <vabr@chromium.org>
Date: Wed Nov 21 10:04:34 2018

Bound the length of phone numbers parsed

Autofill's ParsePhoneNumber parses strings to extract valid phone
numbers. This is a considerable effort (including regex matching) and
a potential attack surface for malicious inputs.

Phone numbers are not arbitrarily long, so exceedingly large inputs
for ParsePhoneNumber signal an error or an attack. They are also
expensive to parse.

Thus, this CL introduces an upper bound on the input length, and the
parser will refuse inputs exceeding this bound.

As a side effect, the autofill_phone_number_i18n_fuzzer will stop
timing out on large inputs.

The CL also wraps the phone number unittest in the anonymous
namespace: This is to prevent potential name clashes for the newly
introduced GenerateTooLongString helper method, but is in general
a good thing to do in unittests, which are never meant to be
exported beyond their own file.

Bug:  901675 
Change-Id: Ie3b069f846288ccd7f11fbce98c54669a55f980f
Reviewed-on: https://chromium-review.googlesource.com/c/1343007
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: Roger McFarlane <rogerm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609979}
[modify] https://crrev.com/f2b4cc12fb1ef8ae8fdbbc8d138682c9c12cd466/components/autofill/core/browser/phone_number_i18n.cc
[modify] https://crrev.com/f2b4cc12fb1ef8ae8fdbbc8d138682c9c12cd466/components/autofill/core/browser/phone_number_i18n.h
[modify] https://crrev.com/f2b4cc12fb1ef8ae8fdbbc8d138682c9c12cd466/components/autofill/core/browser/phone_number_i18n_unittest.cc

Status: Fixed (was: Started)
Project Member

Comment 13 by ClusterFuzz, Nov 22

Labels: OS-Mac
Project Member

Comment 14 by ClusterFuzz, Nov 22

ClusterFuzz has detected this issue as fixed in range 609966:609980.

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

Fuzzer: libFuzzer_autofill_phone_number_i18n_fuzzer
Job Type: libfuzzer_chrome_msan
Platform Id: linux

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

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_msan&range=600499:600510
Fixed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_msan&range=609966:609980

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

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reference.md 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 15 by ClusterFuzz, Nov 22

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

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Cc: -vabr@chromium.org

Sign in to add a comment