Automatically adding ccs based on OWNERS file / target commit history.

If this is incorrect, please add ClusterFuzz-Wrong label.

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!

ClusterFuzz testcase 5727911024525312 appears to be flaky, updating reproducibility label.

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.

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.

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)."

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?

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?

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?

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

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.

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.

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.

Attached.

Deleted: 906338.fuzz 598 bytes

906338.fuzz 598 bytes Download

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?

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

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

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.

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

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.

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

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

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

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.

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.