commit 6e3a40600f246bd8cff2367e3805ce53466cb349
author huyna <huyna89@gmail.com> Thu Jul 25 20:41:23 2019 +0000
committer Chromium commit bot <commit-bot@chromium.org> Thu Jul 25 20:41:23 2019 +0000
tree 9b8fd5ab032f8733af469270a8622f29ca3e5d82
parent 7734ba9a7d52f1e4f14fb7a3c84b282684758519

Prevent an UAF in CPDFSDK_AnnotHandlerMgr::GetNextAnnot().

Calling CXFA_FFPageView::CreateWidgetIterator() can trigger JS and
invalidate CPDFSDK_Annot objects. Use ObservedPtr to check for
CPDFSDK_Annot destruction to catch this.

Bug: chromium:980161
Change-Id: Ifdb926be75c129d7b4a05c3b1fb8c747ef352e71
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/58171
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>

AUTHORS

diff --git a/AUTHORS b/AUTHORS
index 8fe2b46..bd3e4bd 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -20,6 +20,7 @@
 Finnur Thorarinsson <finnur@chromium.org>
 GiWan Go <gogil@stealien.com>
 Henrique Nakashima <hnakashima@chromium.org>
+Huy Ngo <huyna89@gmail.com>
 Jiang Jiang <jiangj@opera.com>
 Jochen Eisinger <jochen@chromium.org>
 John Abd-El-Malek <jam@chromium.org>

fpdfsdk/cpdfsdk_annothandlermgr.cpp

diff --git a/fpdfsdk/cpdfsdk_annothandlermgr.cpp b/fpdfsdk/cpdfsdk_annothandlermgr.cpp
index a16aeba..b1a31fe 100644
--- a/fpdfsdk/cpdfsdk_annothandlermgr.cpp
+++ b/fpdfsdk/cpdfsdk_annothandlermgr.cpp
@@ -229,7 +229,7 @@
       CPDFSDK_FormFillEnvironment::IsALTKeyDown(nFlag)) {
     return GetAnnotHandler(pAnnot)->OnKeyDown(pAnnot, nKeyCode, nFlag);
   }
-
+  ObservedPtr<CPDFSDK_Annot> pObservedAnnot(pAnnot);
   CPDFSDK_PageView* pPage = pAnnot->GetPageView();
   CPDFSDK_Annot* pFocusAnnot = pPage->GetFocusAnnot();
   if (pFocusAnnot && (nKeyCode == FWL_VKEY_Tab)) {
@@ -241,6 +241,10 @@
     }
   }
 
+  // Check |pAnnot| again because JS may have destroyed it in |GetNextAnnot|
+  if (!pObservedAnnot)
+    return false;
+
   return GetAnnotHandler(pAnnot)->OnKeyDown(pAnnot, nKeyCode, nFlag);
 }
 
@@ -310,6 +314,7 @@
 CPDFSDK_Annot* CPDFSDK_AnnotHandlerMgr::GetNextAnnot(CPDFSDK_Annot* pSDKAnnot,
                                                      bool bNext) {
 #ifdef PDF_ENABLE_XFA
+  ObservedPtr<CPDFSDK_Annot> pObservedAnnot(pSDKAnnot);
   CPDFSDK_PageView* pPageView = pSDKAnnot->GetPageView();
   CPDFXFA_Page* pPage = pPageView->GetPDFXFAPage();
   if (pPage && !pPage->AsPDFPage()) {
@@ -319,7 +324,9 @@
             XFA_TRAVERSEWAY_Tranvalse, XFA_WidgetStatus_Visible |
                                            XFA_WidgetStatus_Viewable |
                                            XFA_WidgetStatus_Focused));
-    if (!pWidgetIterator)
+
+    // Check |pSDKAnnot| again because JS may have destroyed it
+    if (!pObservedAnnot || !pWidgetIterator)
       return nullptr;
     if (pWidgetIterator->GetCurrentWidget() != pSDKAnnot->GetXFAWidget())
       pWidgetIterator->SetCurrentWidget(pSDKAnnot->GetXFAWidget());