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

Issue metadata

Status: Verified
Owner:
Closed: Dec 14
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Chrome
Pri: 2
Type: Bug



Sign in to add a comment
link

Issue 906338: Timeout in pdf_font_fuzzer

Reported by ClusterFuzz, Nov 17 Project Member

Issue description

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

Fuzzer: libFuzzer_pdf_font_fuzzer
Job Type: libfuzzer_chrome_asan
Platform Id: linux

Crash Type: Timeout (exceeds 25 secs)
Crash Address: 
Crash State:
  pdf_font_fuzzer
  
Sanitizer: address (ASAN)

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

Issue filed automatically.

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

Comment 1 by ClusterFuzz, Nov 17

Project Member
Cc: tsepez@chromium.org npm@chromium.org weili@chromium.org jochen@chromium.org jam@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.

Comment 2 by kkaluri@chromium.org, Nov 20

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!

Comment 3 by npm@chromium.org, Nov 22

Components: Internals>Plugins>PDF

Comment 4 by ClusterFuzz, Dec 1

Project Member
Labels: -Reproducible Unreproducible
ClusterFuzz testcase 5727911024525312 appears to be flaky, updating reproducibility label.

Comment 5 by infe...@chromium.org, Dec 1

Labels: -Unreproducible Reproducible
Please ignore the last comment about testcase being unreproducible. The testcase is still reproducible. This happened due to a code refactoring on ClusterFuzz side, and the underlying root cause is now fixed. Resetting the label back to Reproducible. Sorry about the inconvenience caused from these incorrect notifications.

Comment 6 by manoranjanr@google.com, Dec 5

Cc: thestig@chromium.org
Labels: -CF-NeedsTriage

Comment 7 by ClusterFuzz, Dec 7

Project Member
Labels: OS-Chrome

Comment 8 by thestig@chromium.org, Dec 8

Cc: -jochen@chromium.org -jam@chromium.org -weili@chromium.org drott@chromium.org bunge...@chromium.org
In a normal build, a lot of the time is spent in TT_RunIns(). Is this something we can fix in FreeType? The input is 598 bytes and it takes a ~5 seconds to run. In the fuzzer, with optimize_for_fuzzing set, it actually slower and takes ~40 seconds to run.

Comment 9 by drott@chromium.org, Dec 11

I am not sure what kind of font the fuzzer produced here but it looks like it is stress testing the TrueType hinting instructions interpreter and spending a lot of time in executing hinting instructions? It's a useful test - but if we timeout, it might take a bit too long? 

We have a configuration value for the max number of opcodes:
https://cs.chromium.org/chromium/src/third_party/freetype/include/freetype-custom-config/ftoption.h?sq=package:chromium&dr=CSs&g=0&l=718

Is this already configured for the fuzzer build?

From the documentation for this setting:
"You don't want to change this except for very special situations (e.g., making a library fuzzer spend less time to handle broken fonts)."

Comment 10 by thestig@chromium.org, Dec 13

The fuzzer is using TT_CONFIG_OPTION_MAX_RUNNABLE_OPCODES from ftoption.h. To have an effect on this bug, I have to set it to a very low value like 10. Is that a reasonable value for a fuzzer or is that too low?

Comment 11 by drott@chromium.org, Dec 13

Cc: behdad@chromium.org
That seems low to me - I think this would effectly prevent TrueType instruction interpretation being fuzzed in a meaningful way. I don't have a good number for what the average opcode depth for hinting instructions is? I think FreeType has configured a public fuzzer as well - perhaps they have a tuned value that we can use - Behdad?

Comment 12 by drott@chromium.org, Dec 13

Cc: lemzw...@googlemail.com
https://github.com/freetype/freetype2-testing/blob/master/fuzzing/settings/freetype2/ftoption.patch Seems contain their ftoption settings for fuzzing, but I am not sure the OSS Fuzz fuzzer hits their TrueType hinting instructions interpreter?

Werner, is OSS fuzz fuzzing the TrueType instructions interpreter? Would you recommend adjusting TT_CONFIG_OPTION_MAX_RUNNABLE_OPCODES when fuzzing?

Comment 13 by behdad@chromium.org, Dec 13

We had similar issue in HarfBuzz.  We used to set limits low for fuzzing.  I recently changed that to always fuzz exactly the code that is used by clients.  That's better testing strategy and makes reproducing fuzzer-found issues easier.  Also, what's too slow for fuzzer, is too slow for a web page as well...

Comment 14 by thestig@chromium.org, Dec 13

Are there opportunities to significantly speed up the code being exercised, or do sanity checks to return early? e.g. in the sfntly fuzzer, if the header claims there are N elements of size M, but the file size is smaller than N * M, then we know the header is invalid.

Comment 15 by behdad@chromium.org, Dec 13

In my experience, making it harder for fuzzer to discover is just hiding the bug.  We should, instead, fix the root cause.  In this case, you said test case 5 seconds to run in a normal build.  That's unacceptable.  It should be fixed by lowering the instruction count.  Loading no glyphs should exceed a few milliseconds.

Comment 16 by lemzw...@googlemail.com, Dec 13

As usual with Chromium fuzzer reports, I don't have permission to access the testcase...  Can someone either give me permissions, or send me the testcase, please?

Yes, the TrueType bytecode interpreter gets fuzzed, and value 10 for TT_CONFIG_OPTION_MAX_RUNNABLE_OPCODES is definitely too small.  I need to see the offending bytecode.

Comment 17 by thestig@chromium.org, Dec 13

Components: Blink>Fonts
Labels: -Pri-1 -M-72 Pri-2
Status: Available (was: Untriaged)
Attached.
906338.fuzz
598 bytes Download

Comment 18 by lemzw...@googlemail.com, Dec 13

Thanks.

What is this file?  A (distorted) font?  FreeType refuses to open it...

Can you extract the buffer contents of Chromium (or whatever you use for fuzzing) that is actually sent to FreeType?

Comment 19 by thestig@chromium.org, Dec 13

It's input to pdf_font_fuzzer. I need to figure out where that data goes from PDFium into FreeType...

Comment 20 by thestig@chromium.org, Dec 13

pdf_font_fuzzer consumes the first 2 bytes, and passes the rest to FT_New_Memory_Face().

Comment 21 by lemzw...@googlemail.com, Dec 14

OK, thanks.  After removing the first two bytes, FreeType loads the file.

Using current git of FreeType, I'm not able to reproduce any timeout.  Only the glyphs with indices 8 and 55 have (identical) bytecode; after processing 94 bytes, FreeType's bytecode interpreter (correctly) aborts due to invalid opcode 0x8F.  And for the protocol, FreeType sets the maximum number of allowed loops to 180 to prevent overlong execution, but there is no loop.

Comment 22 by thestig@chromium.org, Dec 14

Thanks for trying this out. I took a look again and it may be because PDFium is calling FT_Load_Glyph() too many times.

Comment 23 by thestig@chromium.org, Dec 14

Owner: thestig@chromium.org
Status: Started (was: Available)
Sorry for all the noise. It was not clear from the perf graphs what was happening. https://pdfium-review.googlesource.com/c/pdfium/+/47282/ should fix it.

Comment 24 by drott@google.com, Dec 14

No need - no worries at all - thanks for working on a solution!

Comment 25 by bugdroid1@chromium.org, Dec 14

Project Member
The following revision refers to this bug:
  https://pdfium.googlesource.com/pdfium/+/623e636edcd5d8582f7358f7b2f9a4f9636a899e

commit 623e636edcd5d8582f7358f7b2f9a4f9636a899e
Author: Lei Zhang <thestig@chromium.org>
Date: Fri Dec 14 17:27:51 2018

Call GetGlyphWidth() fewer times in LoadCompositeFont().

If LoadCompositeFont() already knows the width for a given glyph index,
avoid calling CFX_Font::GetGlyphWidth() to calculate it again. Repeated
calls to GetGlyphWidth(), which calls FT_Load_Glyph(), can be very slow.

BUG= chromium:906338 

Change-Id: Ifb3ff027f0953a34d1275fdc97a972dbe89ad473
Reviewed-on: https://pdfium-review.googlesource.com/c/47282
Reviewed-by: Nicolás Peña Moreno <npm@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>

[modify] https://crrev.com/623e636edcd5d8582f7358f7b2f9a4f9636a899e/fpdfsdk/fpdf_edittext.cpp

Comment 26 by thestig@chromium.org, Dec 14

Status: Fixed (was: Started)

Comment 27 by bugdroid1@chromium.org, Dec 14

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/39bf42e890c33a2e7f5c42f2d89d1b3b70c3382b

commit 39bf42e890c33a2e7f5c42f2d89d1b3b70c3382b
Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Date: Fri Dec 14 18:53:50 2018

Roll src/third_party/pdfium caacc5ffb136..623e636edcd5 (1 commits)

https://pdfium.googlesource.com/pdfium.git/+log/caacc5ffb136..623e636edcd5


git log caacc5ffb136..623e636edcd5 --date=short --no-merges --format='%ad %ae %s'
2018-12-14 thestig@chromium.org Call GetGlyphWidth() fewer times in LoadCompositeFont().


Created with:
  gclient setdep -r src/third_party/pdfium@623e636edcd5

The AutoRoll server is located here: https://autoroll.skia.org/r/pdfium-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.



BUG= chromium:906338 
TBR=dsinclair@chromium.org

Change-Id: I2d697717edf417ce048fea16fdeb833b7760ef35
Reviewed-on: https://chromium-review.googlesource.com/c/1378205
Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#616766}
[modify] https://crrev.com/39bf42e890c33a2e7f5c42f2d89d1b3b70c3382b/DEPS

Comment 28 by ClusterFuzz, Dec 14

Project Member
ClusterFuzz has detected this issue as fixed in range 616763:616770.

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

Fuzzer: libFuzzer_pdf_font_fuzzer
Fuzz target binary: pdf_font_fuzzer
Job Type: libfuzzer_chrome_asan
Platform Id: linux

Crash Type: Timeout (exceeds 25 secs)
Crash Address: 
Crash State:
  pdf_font_fuzzer
  
Sanitizer: address (ASAN)

Fixed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_asan&range=616763:616770

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

See https://github.com/google/clusterfuzz-tools for instructions to reproduce this bug locally.

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

Comment 29 by ClusterFuzz, Dec 14

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

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

Sign in to add a comment