This issue is a security regression. If you are not able to fix this quickly, please revert the change that introduced it.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Doesn't seem interesting from a security standpoint since we're failing on a checked cast, but still a bug.

Is it really P1 then? Have we looked on the crash server to see how often / if we actually hit this in real life? If it's only happening with the fuzzer, do we want to just let it be? Or is it better to always handle this more gracefully?

I'm also tempted to not fix this for now. Not a security issue, and not a common one (the method is only called using values obtained from freetype, so the font has to be damaged for the crash to happen). Fixing it by adding CheckedNumeric or manually bounding the double slows down PDFium without much gain.

Freetype's horiBearingX glyph metric is a large negative value. Is that a bug?

 Issue 734340  has been merged into this issue.

 Issue 734333  has been merged into this issue.

horiBearingX can be a large negative value, yes.  Of course, this happens only for malformed fonts.

I guess we might as well truncate instead of checked_casting, it'll probably be just as fast.

The following revision refers to this bug:
  https://pdfium.googlesource.com/pdfium/+/4b95360b7611aa73f928de2dc47390f78573c6cc

commit 4b95360b7611aa73f928de2dc47390f78573c6cc
Author: Nicolas Pena <npm@chromium.org>
Date: Wed Jun 21 21:14:05 2017

Clamp instead of checked_cast in TT2PDF

Bug:  chromium:727938 
Change-Id: I85fe329c9d19c1dd1303279b0a9aade2fcc3211c
Reviewed-on: https://pdfium-review.googlesource.com/6814
Commit-Queue: Nicolás Peña <npm@chromium.org>
Reviewed-by: dsinclair <dsinclair@chromium.org>

[modify] https://crrev.com/4b95360b7611aa73f928de2dc47390f78573c6cc/core/fpdfapi/font/cpdf_font.cpp

If we clamp to INT_MAX/INT_MIN, then the caller to TT2PDF() can continue and potentially run into integer overflows. So perhaps UBSAN will notice in the near future and yell at us instead. I know TT2PDF() has a bunch of callers, but I wonder if we can detect malformed fonts, and bail out, instead of crashing, or clamping and then trying to continue.

Here's the thing: at this point freetype has already loaded the font and the TT2PDF are just calculating font metrics from the 'valid' font. The callers of this method are not supposed to fail. One thing we can do to avoid UBSAN if it happens later is clamp in a smaller range instead of INT_MIN/INT_MAX.

Also on a related note, last time I tried to be smart about fixing a PDFium security bug I ended up writing a postmortem instead :( Since this isn't even a real security bug, I just followed the straightforward approach.

OK, let's see how the fuzzing goes. We can deal with new fuzzer bugs as they come up and not stress out too much about it now.

ClusterFuzz has detected this issue as fixed in range 481371:481444.

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

Fuzzer: afl_pdf_font_fuzzer
Job Type: afl_chrome_asan
Platform Id: linux

Crash Type: Ill
Crash Address: 0x00000268211f
Crash State:
  HandleFailure<int>
  checked_cast<int,
  CPDF_Font::TT2PDF
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=afl_chrome_asan&range=472885:472933
Fixed: https://clusterfuzz.com/revisions?job=afl_chrome_asan&range=481371:481444

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


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

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