New issue
Advanced search Search tips

Issue 839361 link

Starred by 1 user

Issue metadata

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

Blocking:
issue 62400



Sign in to add a comment

Use-of-uninitialized-value in bool pdfium::base::internal::CheckedMulOp<unsigned int, unsigned int, void>::Do<

Project Member Reported by ClusterFuzz, May 3 2018

Issue description

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

Fuzzer: libFuzzer_pdf_codec_gif_fuzzer
Job Type: libfuzzer_chrome_msan
Platform Id: linux

Crash Type: Use-of-uninitialized-value
Crash Address: 
Crash State:
  bool pdfium::base::internal::CheckedMulOp<unsigned int, unsigned int, void>::Do<
  bool pdfium::base::internal::CheckedLshOp<unsigned int, unsigned char, void>::Do
  pdfium::base::internal::CheckedNumeric<unsigned int>& pdfium::base::internal::Ch
  
Sanitizer: memory (MSAN)

Recommended Security Severity: Medium

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_msan&range=555575:555601

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

Issue filed automatically.

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

Comment 1 by ClusterFuzz, May 3 2018

Components: Internals>Plugins>PDF
Labels: Test-Predator-Auto-Components
Automatically applying components based on crash stacktrace and information from OWNERS files.

If this is incorrect, please apply the Test-Predator-Wrong-Components label.
Project Member

Comment 2 by ClusterFuzz, May 3 2018

Cc: dsinclair@chromium.org jam@chromium.org brucedaw...@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.
Project Member

Comment 3 by ClusterFuzz, May 3 2018

Labels: Test-Predator-Auto-Owner
Owner: rharrison@chromium.org
Status: Assigned (was: Untriaged)
Automatically assigning owner based on suspected regression changelist https://pdfium.googlesource.com/pdfium/+/cef665eb28bbf4caaa5625332be8f891e3ec2a8e (Convert hand rolled buffer reader in CFX_MemoryStream).

If this is incorrect, please let us know why and apply the Test-Predator-Wrong-CLs label. If you aren't the correct owner for this issue, please unassign yourself as soon as possible so it can be re-triaged.
Project Member

Comment 4 by sheriffbot@chromium.org, May 3 2018

Labels: M-68
Project Member

Comment 5 by sheriffbot@chromium.org, May 3 2018

Labels: ReleaseBlock-Stable
This is a serious security regression. If you are not able to fix this quickly, please revert the change that introduced it.

If this doesn't affect a release branch, or has not been properly classified for severity, please update the Security_Impact or Security_Severity labels, and remove the ReleaseBlock label. To disable this altogether, apply ReleaseBlock-NA.

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

Comment 6 by sheriffbot@chromium.org, May 3 2018

Labels: Pri-1
Blocking: 62400
Labels: -Security_Impact-Head -ReleaseBlock-Stable Security_Impact-None
Status: Started (was: Assigned)
XFA only
Project Member

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

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

commit ad178855775da18d7abfadce4b64825f3e32adbd
Author: Ryan Harrison <rharrison@chromium.org>
Date: Thu May 03 20:40:35 2018

Invalidate GIF input buffer when moving file cursor backwards

The current implementation of the GIF codec does not handle the file
cursor moving backwards correctly. Specifically the input buffer that
the data is being read into is not invalidated, so if the entirity of
the buffer hasn't been consumed, a chunk of it will be moved to the
front before reading in more data, which is just
incorrect. Additionally, depending on the specific series of
operations, it is possible that the buffer was allocated for more
space then had been read into it and the uninitialized portion at the
end is being copied to the beginning.

The file cursor may move backwards when dealing with an animated gif
or other image with multiple frames, since all of the control data is
read in on load, and future calls specify what frame to fetch. The
code has been changed to treat the input buffer as invalid when moving
the cursor to a frame location, which will bypass any of the
problematic unused saving behaviour. A call to std::min has been added
to prevent allocation of an input buffer larger then the file size.

Additionally this CL refactors GifReadMoreData to be clearer about
what calculations are occuring, since the existing code reuses a
number of vaguely named variables, making it difficult to follow.

BUG= chromium:839348 ,  chromium:839361 

Change-Id: I2865658187bdf30bcad13ef4cac4f51a8966db11
Reviewed-on: https://pdfium-review.googlesource.com/32054
Reviewed-by: Henrique Nakashima <hnakashima@chromium.org>
Commit-Queue: Ryan Harrison <rharrison@chromium.org>

[modify] https://crrev.com/ad178855775da18d7abfadce4b64825f3e32adbd/core/fxcodec/codec/ccodec_progressivedecoder.h
[modify] https://crrev.com/ad178855775da18d7abfadce4b64825f3e32adbd/core/fxcodec/codec/fx_codec_progress.cpp

Status: Fixed (was: Started)
Project Member

Comment 10 by bugdroid1@chromium.org, May 4 2018

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

commit 5cee59c3852ed3c5e078ae0c23fce23a77983bcb
Author: pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Fri May 04 01:01:19 2018

Roll src/third_party/pdfium/ 525147a1f..ad1788557 (5 commits)

https://pdfium.googlesource.com/pdfium.git/+log/525147a1f6d6..ad178855775d

$ git log 525147a1f..ad1788557 --date=short --no-merges --format='%ad %ae %s'
2018-05-03 rharrison Invalidate GIF input buffer when moving file cursor backwards
2018-05-03 tsepez Prove that the memory was good at FPDFBitmap_CreateEx() create time.
2018-05-03 hnakashima Use pointers instead of refs in CXFA_TextLayout params.
2018-05-03 dsinclair [xfa] Verify we can get a font manager before setting up XFA
2018-05-03 dsinclair [xfa] Verify field count before accessing

Created with:
  roll-dep src/third_party/pdfium
BUG= chromium:839348 , chromium:839361 , chromium:838886 , chromium:835693 , chromium:837585 


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: I06ec60f0a34b13f864be053ffe512402c4c8ad7a
Reviewed-on: https://chromium-review.googlesource.com/1043278
Reviewed-by: pdfium-chromium-autoroll <pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: pdfium-chromium-autoroll <pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#555941}
[modify] https://crrev.com/5cee59c3852ed3c5e078ae0c23fce23a77983bcb/DEPS

Project Member

Comment 11 by ClusterFuzz, May 4 2018

ClusterFuzz has detected this issue as fixed in range 555940:555953.

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

Fuzzer: libFuzzer_pdf_codec_gif_fuzzer
Job Type: libfuzzer_chrome_msan
Platform Id: linux

Crash Type: Use-of-uninitialized-value
Crash Address: 
Crash State:
  bool pdfium::base::internal::CheckedMulOp<unsigned int, unsigned int, void>::Do<
  bool pdfium::base::internal::CheckedLshOp<unsigned int, unsigned char, void>::Do
  pdfium::base::internal::CheckedNumeric<unsigned int>& pdfium::base::internal::Ch
  
Sanitizer: memory (MSAN)

Recommended Security Severity: Medium

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_msan&range=555575:555601
Fixed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_msan&range=555940:555953

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

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 12 by ClusterFuzz, May 4 2018

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

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

Comment 13 by sheriffbot@chromium.org, May 4 2018

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Project Member

Comment 14 by sheriffbot@chromium.org, Aug 10

Labels: -Restrict-View-SecurityNotify allpublic
This bug has been closed for more than 14 weeks. Removing security view restrictions.

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

Sign in to add a comment