New issue
Advanced search Search tips

Issue 771479 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

Heap-use-after-free in CPDF_SecurityHandler::~CPDF_SecurityHandler

Project Member Reported by ClusterFuzz, Oct 4 2017

Issue description

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

Fuzzer: corpus_builder_pdf
Job Type: linux_asan_pdfium
Platform Id: linux

Crash Type: Heap-use-after-free READ 1
Crash Address: 0x605000001dd0
Crash State:
  CPDF_SecurityHandler::~CPDF_SecurityHandler
  CPDF_Document::~CPDF_Document
  RenderPdf
  
Sanitizer: address (ASAN)

Recommended Security Severity: High

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_pdfium&range=506154:506214

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

Issue filed automatically.

See https://github.com/google/clusterfuzz-tools for more information.
 
Project Member

Comment 1 by sheriffbot@chromium.org, Oct 4 2017

Labels: M-63
Project Member

Comment 2 by sheriffbot@chromium.org, Oct 4 2017

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 3 by sheriffbot@chromium.org, Oct 4 2017

Labels: Pri-1
Components: Internals>Plugins>PDF
Owner: dsinclair@chromium.org
Status: Assigned (was: Untriaged)
Dan, could you please help with triaging this?
Cc: art-sn...@yandex-team.ru
My guess is that this is: https://pdfium-review.googlesource.com/c/pdfium/+/15290 which I don't think will be easily revertable.

art-snake@ can you please take a look? My guess is the ID array is getting destroyed before the SecurityHandler.
Cc: tsepez@chromium.org thestig@chromium.org
(I'm not sure if this is a RB-Stable issue. It's our probe for memory violations which won't trigger in production, it's only on for the *San builds.)

tsepez@ should the RB-Stable get dropped here?
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 4 2017

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

commit adb19b0b11ab16a406d23797564fc0ec3a5da245
Author: Artem Strygin <art-snake@yandex-team.ru>
Date: Wed Oct 04 17:31:05 2017

Fix Heap-use-after-free in CPDF_SecurityHandler::~CPDF_SecurityHandler.


The CPDF_SecurityHandler contains unowned reference to "ID" array, which is owned by main trailer.
Main trailer is owned by CPDF_Parser::m_TrailerData

To fix this issue 
set m_TrailerData before m_pSecurityHandler(CPDF_SecurityHandler) in CPDF_Parser members list.

Bug:  chromium:771479 
Change-Id: I38413ba16b1454ac775c8a07b126fa3b86714c1b
Reviewed-on: https://pdfium-review.googlesource.com/15430
Reviewed-by: dsinclair <dsinclair@chromium.org>
Commit-Queue: Art Snake <art-snake@yandex-team.ru>

[modify] https://crrev.com/adb19b0b11ab16a406d23797564fc0ec3a5da245/core/fpdfapi/parser/cpdf_parser.cpp
[modify] https://crrev.com/adb19b0b11ab16a406d23797564fc0ec3a5da245/core/fpdfapi/parser/cpdf_parser.h

Status: Fixed (was: Assigned)
Project Member

Comment 9 by ClusterFuzz, Oct 4 2017

Labels: ClusterFuzz-Top-Crash ReleaseBlock-Beta
Testcase 4512946976980992 is a top crash on ClusterFuzz for linux platform. Please prioritize fixing this crash.

Marking this crash as a Beta release blocker.

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

Comment 10 by sheriffbot@chromium.org, Oct 5 2017

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Owner: rharrison@chromium.org
Taking ownership temporarily to see if it reproduces on HEAD
Owner: dsinclair@chromium.org
Status: Assigned (was: Fixed)
This is still reproducing on HEAD
Owner: hnakashima@chromium.org
M63 is branching on this Thursday (10/12) and M63 beta promotion is coming very soon. Your bug is labelled as Beta ReleaseBlock, pls make sure to land the fix ASAP. Thank you.
Project Member

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

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

commit fb6165ff8f8ad1d7725f63e509eb7f7543df231e
Author: Henrique Nakashima <hnakashima@chromium.org>
Date: Tue Oct 10 20:23:26 2017

Fix dangling pointer to ID array in CPDF_SecurityHandler.

This was caused by breaking the reference from CPDF_SecurityHandler to
CPDF_Parser in https://pdfium-review.googlesource.com/c/pdfium/+/15290

The reference was replaced with a reference to the ID Array and a copy
of the password. The issue is that when parsing PDFs with multiple
trailers, the trailer containing the ID array may be replaced and
destroyed in CPDF_Parser::TrailerData::SetMainTrailer() after being
passed to CPDF_SecurityHandler, which would then have a dangling
pointer to it.

This CL changes the CPDF_SecurityHandler to hold a copy of the original
file ID instead of all the ID Array.

Bug:  chromium:771479 , chromium:772376 
Change-Id: Id98100502093d890fc2fe6a3da139f910daf38f4
Reviewed-on: https://pdfium-review.googlesource.com/15910
Commit-Queue: Henrique Nakashima <hnakashima@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>

[modify] https://crrev.com/fb6165ff8f8ad1d7725f63e509eb7f7543df231e/core/fpdfapi/parser/cpdf_security_handler.cpp
[modify] https://crrev.com/fb6165ff8f8ad1d7725f63e509eb7f7543df231e/core/fpdfapi/parser/cpdf_security_handler.h

Status: Fixed (was: Assigned)
Project Member

Comment 17 by ClusterFuzz, Oct 11 2017

ClusterFuzz has detected this issue as fixed in range 507774:507876.

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

Fuzzer: corpus_builder_pdf
Job Type: linux_asan_pdfium
Platform Id: linux

Crash Type: Heap-use-after-free READ 1
Crash Address: 0x605000001dd0
Crash State:
  CPDF_SecurityHandler::~CPDF_SecurityHandler
  CPDF_Document::~CPDF_Document
  RenderPdf
  
Sanitizer: address (ASAN)

Recommended Security Severity: High

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_pdfium&range=506154:506214
Fixed: https://clusterfuzz.com/revisions?job=linux_asan_pdfium&range=507774:507876

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

See https://github.com/google/clusterfuzz-tools 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 18 by ClusterFuzz, Oct 11 2017

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

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Labels: -ReleaseBlock-Beta -ReleaseBlock-Stable
Project Member

Comment 20 by sheriffbot@chromium.org, Jan 17 2018

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
Project Member

Comment 21 by sheriffbot@chromium.org, Mar 27 2018

Labels: -Security_Impact-Head -M-63 M-65 Security_Impact-Stable

Sign in to add a comment