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

Issue 783022 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug

Blocking:
issue 62400



Sign in to add a comment

Abrt in pdfium::base::PartitionExcessiveAllocationSize

Project Member Reported by ClusterFuzz, Nov 9 2017

Issue description

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

Fuzzer: libFuzzer_pdf_codec_tiff_fuzzer
Job Type: libfuzzer_chrome_ubsan
Platform Id: linux

Crash Type: Abrt
Crash Address: 0x03e90000085b
Crash State:
  pdfium::base::PartitionExcessiveAllocationSize
  pdfium::base::PartitionReallocGeneric
  _TIFFCheckRealloc
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_ubsan&range=508791:508824

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

Issue filed automatically.

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reference.md for more information.
 
Components: Internals>Plugins>PDF
Cc: msrchandra@chromium.org thestig@chromium.org pnangunoori@chromium.org
Labels: M-64 Test-Predator-Wrong
Owner: palmer@chromium.org
Status: Assigned (was: Untriaged)
Predator and CL could not provide any possible suspects.
Using the code search for the file, “partition_alloc.cc” assigning to concern owner from GIT blame.
Suspecting Commit#
https://pdfium.googlesource.com/pdfium.git/+/79e548eb98caefd3ea0f0e4806a7abca6654e7dc

@palmer -- Could you please look into this issue, kindly reassign if it has nothing to do with your changes.
Thank You.

Blocking: 62400
Cc: npm@chromium.org
Labels: -Pri-1 -M-64 Pri-2
Owner: ----
Status: Available (was: Assigned)
Someone needs to fix this in libtiff. This is XFA only, BTW.
Owner: rharrison@chromium.org
Status: Assigned (was: Available)
thestig@, what were you thinking of for a fix here? Debugging the code, it looks like this file is causing a very large allocation, which causes the PartitionAllocator to OOM.

Are you thinking of changing the higher level algorithm, so that it doesn't need such a large allocation? or were you thinking having libtiff detect it is going to OOM, and instead fail the decode there?
Status: Started (was: Assigned)
Cc: palmer@chromium.org
Maybe modify _TIFFrealloc() to fail by returning nullptr if we know PA is going to fail on the large allocation?
#7: That SGTM.
Project Member

Comment 9 by bugdroid1@chromium.org, May 8 2018

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

commit e6ead58ddb568705fbf173aca4ee1abf0da94188
Author: Ryan Harrison <rharrison@chromium.org>
Date: Tue May 08 17:51:31 2018

Add Realloc version of PartitionAllocGenericFlags

This adds in a new function, PartitionRellocGenericFlags, which allows
invoking realloc with passed in flags. This in turn allows for
controlling behaviours like if the call should return null or OOM when
unable to satisfy a request. The existing method
PartitionRootGeneric::Realloc has been refactored to use this new
function with no flags, so there only needs to be one implementation.

BUG= chromium:783022 

Change-Id: Iee783874b16b56efdaa1ad0bd0dd8f0b485c68f3
Reviewed-on: https://chromium-review.googlesource.com/1044971
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Albert J. Wong <ajwong@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556862}
[modify] https://crrev.com/e6ead58ddb568705fbf173aca4ee1abf0da94188/base/allocator/partition_allocator/partition_alloc.cc
[modify] https://crrev.com/e6ead58ddb568705fbf173aca4ee1abf0da94188/base/allocator/partition_allocator/partition_alloc.h
[modify] https://crrev.com/e6ead58ddb568705fbf173aca4ee1abf0da94188/base/allocator/partition_allocator/partition_alloc_unittest.cc

Status: Fixed (was: Started)
Project Member

Comment 11 by bugdroid1@chromium.org, May 16 2018

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

commit 98ec53359b8e61e717440f280d3fcc101fe140bb
Author: Ryan Harrison <rharrison@chromium.org>
Date: Wed May 16 19:19:22 2018

Add support for PartionRealloc to return nullptr

Currently the PartitionRealloc code path will only exit, with no
option to return nullptr on failure, unlike PartitionAlloc code path.
This CL refactors the realloc code path to be similar to alloc code
path, following the upstream patch:
https://chromium-review.googlesource.com/c/chromium/src/+/1044971

This also changes the version of realloc exposed to third party C libs
to have the nullptr behaviour, like the exposed version of alloc.

This CL is a redo of
https://pdfium-review.googlesource.com/c/pdfium/+/31990

BUG= chromium:783022 

Change-Id: Ib1b659079585dfd0423d683b8a2c7b6758a22a01
Reviewed-on: https://pdfium-review.googlesource.com/32613
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Chris Palmer <palmer@chromium.org>

[modify] https://crrev.com/98ec53359b8e61e717440f280d3fcc101fe140bb/third_party/base/allocator/partition_allocator/partition_alloc.cc
[modify] https://crrev.com/98ec53359b8e61e717440f280d3fcc101fe140bb/core/fxcrt/fx_memory.h
[modify] https://crrev.com/98ec53359b8e61e717440f280d3fcc101fe140bb/third_party/base/allocator/partition_allocator/partition_alloc.h
[modify] https://crrev.com/98ec53359b8e61e717440f280d3fcc101fe140bb/core/fxcrt/fx_memory.cpp

Project Member

Comment 12 by bugdroid1@chromium.org, May 17 2018

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

commit c59cbcfced893546b538e3d3aa8ea29b8887341b
Author: pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Thu May 17 00:14:45 2018

Roll src/third_party/pdfium/ 33515fbec..98ec53359 (4 commits)

https://pdfium.googlesource.com/pdfium.git/+log/33515fbec463..98ec53359b8e

$ git log 33515fbec..98ec53359 --date=short --no-merges --format='%ad %ae %s'
2018-05-16 rharrison Add support for PartionRealloc to return nullptr
2018-05-16 thestig Update third_party/base/bits.h.
2018-05-16 tsepez Remove some more unused #defines
2018-05-16 tsepez Move FX_LBUN shorthand codes from .h to .cpp file

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


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

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.


TBR=dsinclair@chromium.org

Change-Id: I6a02d43557a7e40c91e953fb0a1324bb44d7710d
Reviewed-on: https://chromium-review.googlesource.com/1062772
Commit-Queue: pdfium-chromium-autoroll <pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Reviewed-by: pdfium-chromium-autoroll <pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#559357}
[modify] https://crrev.com/c59cbcfced893546b538e3d3aa8ea29b8887341b/DEPS

Project Member

Comment 13 by ClusterFuzz, May 17 2018

ClusterFuzz has detected this issue as fixed in range 559337:559361.

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

Fuzzer: libFuzzer_pdf_codec_tiff_fuzzer
Job Type: libfuzzer_chrome_ubsan
Platform Id: linux

Crash Type: Abrt
Crash Address: 0x053900000838
Crash State:
  pdfium::base::PartitionExcessiveAllocationSize
  pdfium::base::PartitionReallocGeneric
  _TIFFCheckRealloc
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_ubsan&range=508791:508824
Fixed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_ubsan&range=559337:559361

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

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 14 by ClusterFuzz, May 17 2018

Labels: ClusterFuzz-Verified
Status: Verified (was: Fixed)
ClusterFuzz testcase 5770563724509184 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