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

Issue 752920 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug

Blocking:
issue 62400



Sign in to add a comment

Out-of-memory in pdf_fm2js_fuzzer

Project Member Reported by ClusterFuzz, Aug 7 2017

Issue description

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

Fuzzer: libFuzzer_pdf_fm2js_fuzzer
Job Type: libfuzzer_chrome_msan
Platform Id: linux

Crash Type: Out-of-memory (exceeds 2048 MB)
Crash Address: 
Crash State:
  pdf_fm2js_fuzzer
  
Sanitizer: memory (MSAN)

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_msan&range=395689:395794

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


Issue filed automatically.

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md for more information.
 
Cc: msrchandra@chromium.org
Labels: M-62 Test-Predator-Wrong
Owner: dsinclair@chromium.org
Status: Assigned (was: Untriaged)
Predator and CL did not provide any possible suspects.
Using Code Search for the file, "pdf_fm2js_fuzzer" assigning to the concern owner who might be related.

@dsinclair -- Could you please look into the issue, kindly re-assign if this is not related to your changes.
Thank You.

Blocking: 62400
Components: Internals>Plugins>PDF
Owner: rharrison@chromium.org
Labels: -M-62 Security_Impact-None
Status: Started (was: Assigned)
Dug into this a bit. Looks like the FormCalc parse is fine. It is when it tries to emit the JS that it bombs out. There should be a hard limit of 256 MB on the size of the string, which a) seems ludicriously high, b) isn't getting hit. So either there is something like 2 GB of overhead when emitting or this check isn't being hit everywhere.

Comment 7 by mmoroz@chromium.org, Oct 24 2017

For more information, please see https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reference.md.

The link referenced in the description is no longer valid.

(bulk edit)
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 26 2017

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

commit efc879d226e98ddad36704d78f52037ccf4369dc
Author: Ryan Harrison <rharrison@chromium.org>
Date: Thu Oct 26 15:16:56 2017

Consistently apply JS size check

This CL makes the use of CXFA_IsTooBig consistent and universal across
all of the ToJavascript and related methods. Previously this method
was only applied in some calls. It is now being tested as a 
precondition and postcondition on every call and as a post condition 
for the entire translation process.

There are some size checks within methods that potentially generate
large amounts of JS that have been left in.

BUG= chromium:752920 

Change-Id: I1a8bfe21e0a17c2e47592fc6017060243674f1bc
Reviewed-on: https://pdfium-review.googlesource.com/16812
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>

[modify] https://crrev.com/efc879d226e98ddad36704d78f52037ccf4369dc/xfa/fxfa/fm2js/cxfa_fmexpression.cpp
[modify] https://crrev.com/efc879d226e98ddad36704d78f52037ccf4369dc/xfa/fxfa/fm2js/cxfa_fmsimpleexpression.cpp
[modify] https://crrev.com/efc879d226e98ddad36704d78f52037ccf4369dc/xfa/fxfa/fm2js/cxfa_fm2jscontext.cpp

Status: WontFix (was: Started)
The above patch improves detecting excessively large JS, but the OOM is still being hit. If the memory limit is increased to 4 gigs, it correctly recognizes that the JS is too large and thus bails. Thus the code is behaving 'correctly'.

256 MB is the correct limit, since that is a limit on JS size in V8.

There is a another issue with regards to poor JS generation, since the JS is increasing in size exponentially, double with each nested function call. I have created a new bug to track fixing that issues. https://bugs.chromium.org/p/pdfium/issues/detail?id=924, though it will likely require significant rework of the JS generation code.

Marking this as WontFix, since the code does correctly catch the JS too big issue, just requires a bit more memory.
Project Member

Comment 10 by bugdroid1@chromium.org, Oct 27 2017

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

commit 0c383a4f0d4af3e7d402d55909ed2d71b48031e2
Author: pdfium-deps-roller@chromium.org <pdfium-deps-roller@chromium.org>
Date: Fri Oct 27 17:44:13 2017

Roll src/third_party/pdfium/ 78b334a82..42613d896 (16 commits)

https://pdfium.googlesource.com/pdfium.git/+log/78b334a824c4..42613d8961fe

$ git log 78b334a82..42613d896 --date=short --no-merges --format='%ad %ae %s'
2017-10-27 hnakashima Revert "Remove ContrastAdjust()."
2017-10-26 dsinclair Remove unneeded DefineJSObject param
2017-10-26 dsinclair Make spec arrays const
2017-10-26 dsinclair Cleanup statics in JS classes
2017-10-26 dsinclair Remove g_pClassName
2017-10-26 dsinclair Cleanup JS define methods
2017-10-26 dsinclair Remove methods for empty const/method/property arrays
2017-10-26 dsinclair Remove JS macros
2017-10-26 dsinclair Remove CJS_Array
2017-10-25 dsinclair Rename JS_EventHandler
2017-10-25 dsinclair Move global.{h|cpp} to match class
2017-10-25 dsinclair Rename Annot to cjs_annot
2017-10-25 dsinclair Split Consts into individual files
2017-10-26 hnakashima Add option to regenerate only platform-specific expected pngs.
2017-10-26 hnakashima Remove ContrastAdjust().
2017-10-26 rharrison Consistently apply JS size check

Created with:
  roll-dep src/third_party/pdfium
BUG= 752920 


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

If the roll is causing failures, see:
http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls


TBR=dsinclair@chromium.org

Change-Id: Ibda7ca437c3f8cf930d15b5a59c819f9a8a5b613
Reviewed-on: https://chromium-review.googlesource.com/741922
Reviewed-by: <pdfium-deps-roller@chromium.org>
Commit-Queue: <pdfium-deps-roller@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512226}
[modify] https://crrev.com/0c383a4f0d4af3e7d402d55909ed2d71b48031e2/DEPS

Sign in to add a comment