New issue
Advanced search Search tips
Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug-Security



Sign in to add a comment
link

Issue 760455: Security: Use-after-free in CPWL_Edit::OnKillFocus()

Reported by manhluat...@gmail.com, Aug 30 2017

Issue description

Tested on

Chromium	62.0.3197.0 (Developer Build) (64-bit)
Revision	e5aab5c2b9d9e85ffccbdb3571e9f7d218e95601-refs/heads/master@{#497322}
/w ASAN

It should work on M60 61 as well.

----------------------------------------------------------

relevant report you may want to check: https://bugs.chromium.org/p/chromium/issues/detail?id=737023


https://cs.chromium.org/chromium/src/third_party/pdfium/fpdfsdk/pwl/cpwl_edit.cpp?q=cpwl_edit.cpp:351&sq=package:chromium&l=342

[...]
void CPWL_Edit::OnKillFocus() {
  CPWL_ScrollBar* pScroll = GetVScrollBar();
  if (pScroll && pScroll->IsVisible()) {
    pScroll->SetVisible(false);
    Move(m_rcOldWindow, true, true);
  }

  m_pEdit->SelectNone();
  SetCaret(false, CFX_PointF(), CFX_PointF()); <--- I've managed to run AAction, to destroy the pdfwindow in middle of |SetCaret|. then uaf occurs at the next lines.
  SetCharSet(FX_CHARSET_ANSI);
  m_bFocus = false;
}
[...]

To destroy the pdf window |ResetPDFWindow|, I've been using |setFocus()|
https://cs.chromium.org/chromium/src/third_party/pdfium/fpdfsdk/formfiller/cffl_interactiveformfiller.cpp?type=cs&q=CFFL_InteractiveFormFiller::OnSetFocus&sq=package:chromium&l=399
 
poc.pdf
4.0 KB Download

Comment 1 by manhluat...@gmail.com, Aug 30 2017

The following is another PoC, which I spray heap to take over freed object, with arbitrary value.

Tested on Chromium local build on Linux.
Screenshot from 2017-08-28 15-54-45.png
337 KB View Download
poc_2.pdf
4.2 KB Download
poc_gdb.txt
7.1 KB View Download

Comment 2 by manhluat...@gmail.com, Aug 30 2017

Just clarify it,

|SetCaret| invokes |InvalidateRect| later, which calls |Form_Invalidate| -> |GetVisiblePages| -> |GetPage| (assume: page X) -> |LoadFXAnnots| -> so I run AAction script at here on |OnFormat| of any annot on the page X.

Comment 3 by elawrence@chromium.org, Aug 30 2017

Components: Internals>Plugins>PDF

Comment 4 by dsinclair@chromium.org, Aug 30 2017

Cc: tsepez@chromium.org
Owner: rharrison@chromium.org
Status: Assigned (was: Unconfirmed)
rharrison@ can you take a look? We probably need to add an ObservedPtr on this in there or something ....

As a note, does this require XFA enabled to trigger?

Comment 5 by manhluat...@gmail.com, Aug 30 2017

No, It doesn't need XFA enabled.

Comment 6 by dominickn@chromium.org, Aug 30 2017

Labels: Security_Severity-High Security_Impact-Stable OS-Chrome OS-Linux OS-Mac OS-Windows Pri-1
Assigning high severity and impact on stable across desktop.

Comment 7 by sheriffbot@chromium.org, Aug 31 2017

Project Member
Labels: M-60

Comment 8 by rharrison@chromium.org, Sep 5 2017

Status: Started (was: Assigned)

Comment 9 by sheriffbot@chromium.org, Sep 6 2017

Project Member
Labels: -M-60 M-61

Comment 10 by rharrison@chromium.org, Sep 12 2017

Status: Assigned (was: Started)
I have looked at this a bit. It is relatively easy to use ObservedPtr to resolve the issue in the specific location that it was reported, but that then causes new locations to appear due to them being masked by the first one.

Getting this nailed down so that all of the underlying locations are caught is going to be non-trivial.

Comment 11 by rharrison@chromium.org, Sep 15 2017

Status: Started (was: Assigned)

Comment 12 Deleted

Comment 13 by manhluat...@gmail.com, Sep 15 2017

@rharrison

You may want to take a look at my latest report

https://bugs.chromium.org/p/chromium/issues/detail?id=765384

Comment 14 by bugdroid1@chromium.org, Sep 15 2017

Project Member
The following revision refers to this bug:
  https://pdfium.googlesource.com/pdfium/+/022d13b85408beb400ce703bb5c59736adea208f

commit 022d13b85408beb400ce703bb5c59736adea208f
Author: Ryan Harrison <rharrison@chromium.org>
Date: Fri Sep 15 18:45:55 2017

Add ObservedPtrs to KillFocus path

This is to prevent use after free issues due to these calls causing
reloads of content that have the side of effect of destroying windows.

BUG= chromium:760455 

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

[modify] https://crrev.com/022d13b85408beb400ce703bb5c59736adea208f/fpdfsdk/pwl/cpwl_wnd.cpp
[modify] https://crrev.com/022d13b85408beb400ce703bb5c59736adea208f/fpdfsdk/pwl/cpwl_edit.cpp

Comment 15 by rharrison@chromium.org, Sep 15 2017

Status: Fixed (was: Started)
Should this be merged anywhere?

Comment 16 by bugdroid1@chromium.org, Sep 15 2017

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/517d4150a14d42f32bfc6bcdafce172ab3a47ff4

commit 517d4150a14d42f32bfc6bcdafce172ab3a47ff4
Author: pdfium-deps-roller@chromium.org <pdfium-deps-roller@chromium.org>
Date: Fri Sep 15 21:21:48 2017

Roll src/third_party/pdfium/ 574756152..bb2f7e73b (2 commits)

https://pdfium.googlesource.com/pdfium.git/+log/574756152de8..bb2f7e73bef4

$ git log 574756152..bb2f7e73b --date=short --no-merges --format='%ad %ae %s'
2017-09-15 thestig Simplify a couple of CXFA_Node methods.
2017-09-15 rharrison Add ObservedPtrs to KillFocus path

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


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: I5fd7a96a9eaa061fe2efe63434646c9edcb22ecc
Reviewed-on: https://chromium-review.googlesource.com/669307
Reviewed-by: <pdfium-deps-roller@chromium.org>
Commit-Queue: <pdfium-deps-roller@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502381}
[modify] https://crrev.com/517d4150a14d42f32bfc6bcdafce172ab3a47ff4/DEPS

Comment 17 by sheriffbot@chromium.org, Sep 16 2017

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

Comment 18 by sheriffbot@chromium.org, Sep 18 2017

Project Member
Labels: Merge-Request-62

Comment 19 by sheriffbot@chromium.org, Sep 18 2017

Project Member
Labels: -Merge-Request-62 Merge-Review-62 Hotlist-Merge-Review
This bug requires manual review: DEPS changes referenced in bugdroid comments.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

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

Comment 20 by abdulsyed@google.com, Sep 18 2017

+awhalley@ - can you please confirm if this is okay to take for M62?

Comment 21 by awhalley@chromium.org, Sep 18 2017

Labels: reward-topanel
abdulsyed@ - yep, good for 62

Comment 22 by abdulsyed@chromium.org, Sep 18 2017

Labels: -Merge-Review-62 Merge-Approved-62
Thanks for checking - approving merge to M62. Branch: 3202

Comment 23 by bugdroid1@chromium.org, Sep 19 2017

Project Member
Labels: -merge-approved-62 merge-merged-3202
The following revision refers to this bug:
  https://pdfium.googlesource.com/pdfium/+/6f7bd9fdc4f4d7f21e468c47ee3e5616330541a6

commit 6f7bd9fdc4f4d7f21e468c47ee3e5616330541a6
Author: Ryan Harrison <rharrison@chromium.org>
Date: Tue Sep 19 14:36:02 2017

[Merge M62] Add ObservedPtrs to KillFocus path

This is to prevent use after free issues due to these calls causing
reloads of content that have the side of effect of destroying windows.

BUG= chromium:760455 

Change-Id: I3f3947be8b32964783abf5577a24ba6a713b3476
Reviewed-on: https://pdfium-review.googlesource.com/14150
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
(cherry picked from commit 022d13b85408beb400ce703bb5c59736adea208f)
Reviewed-on: https://pdfium-review.googlesource.com/14310
Reviewed-by: dsinclair <dsinclair@chromium.org>

[modify] https://crrev.com/6f7bd9fdc4f4d7f21e468c47ee3e5616330541a6/fpdfsdk/pwl/cpwl_wnd.cpp
[modify] https://crrev.com/6f7bd9fdc4f4d7f21e468c47ee3e5616330541a6/fpdfsdk/pwl/cpwl_edit.cpp

Comment 24 by awhalley@chromium.org, Sep 22 2017

Labels: -reward-topanel reward-unpaid reward-3000
*** Boilerplate reminders! ***
Please do NOT publicly disclose details until a fix has been released to all our users. Early public disclosure may cancel the provisional reward. Also, please be considerate about disclosure when the bug affects a core library that may be used by other products. Please do NOT share this information with third parties who are not directly involved in fixing the bug. Doing so may cancel the provisional reward. Please be honest if you have already disclosed anything publicly or to third parties. Lastly, we understand that some of you are not interested in money. We offer the option to donate your reward to an eligible charity. If you prefer this option, let us know and we will also match your donation - subject to our discretion. Any rewards that are unclaimed after 12 months will be donated to a charity of our choosing.
*********************************

Comment 25 by awhalley@google.com, Sep 22 2017

Nice one! The panel decided to award $3,000 for this report. Thanks!

Comment 26 by awhalley@chromium.org, Sep 22 2017

Labels: -reward-unpaid reward-inprocess

Comment 27 Deleted

Comment 28 by awhalley@google.com, Oct 16 2017

Labels: M-62 Release-0-M62

Comment 29 by awhalley@chromium.org, Oct 18 2017

Labels: CVE-2017-5126

Comment 30 by sheriffbot@chromium.org, Dec 23 2017

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

Comment 31 by awhalley@chromium.org, Apr 25 2018

Labels: CVE_description-submitted

Sign in to add a comment