Merge to XFA:Fix heap use after free in CPDFSDK_Annot::GetPDFAnnot.
(cherry picked from commit 9241e5a43990859f6f9a94aaa2c488d0451039e3)
Original Review URL: https://codereview.chromium.org/1312313006 .
(cherry picked from commit 343dbb841f4c12e819932e2b66dd70f817337d97)
Original Review URL: https://codereview.chromium.org/1325533004 .
BUG=507316
TBR=thestig@chromium.org
Review URL: https://codereview.chromium.org/1332653002 .
diff --git a/fpdfsdk/src/fpdfformfill_embeddertest.cpp b/fpdfsdk/src/fpdfformfill_embeddertest.cpp
index 6baad11..56710b9 100644
--- a/fpdfsdk/src/fpdfformfill_embeddertest.cpp
+++ b/fpdfsdk/src/fpdfformfill_embeddertest.cpp
@@ -40,6 +40,18 @@
UnloadPage(page);
}
+TEST_F(FPDFFormFillEmbeddertest, BUG_507316) {
+ EmbedderTestTimerHandlingDelegate delegate;
+ SetDelegate(&delegate);
+
+ EXPECT_TRUE(OpenDocument("testing/resources/bug_507316.pdf"));
+ FPDF_PAGE page = LoadAndCachePage(2);
+ EXPECT_NE(nullptr, page);
+ DoOpenActions();
+ delegate.AdvanceTime(4000);
+ UnloadPage(page);
+}
+
TEST_F(FPDFFormFillEmbeddertest, BUG_514690) {
EXPECT_TRUE(OpenDocument("testing/resources/hello_world.pdf"));
FPDF_PAGE page = LoadPage(0);
diff --git a/fpdfsdk/src/fsdk_mgr.cpp b/fpdfsdk/src/fsdk_mgr.cpp
index 4d3d01f..4a55622 100644
--- a/fpdfsdk/src/fsdk_mgr.cpp
+++ b/fpdfsdk/src/fsdk_mgr.cpp
@@ -664,15 +664,22 @@
}
CPDFSDK_PageView::~CPDFSDK_PageView() {
+ // if there is a focused annot on the page, we should kill the focus first.
+ if (CPDFSDK_Annot* focusedAnnot = m_pSDKDoc->GetFocusAnnot()) {
+ for (int i = 0, count = m_fxAnnotArray.GetSize(); i < count; i++) {
+ CPDFSDK_Annot* pAnnot = (CPDFSDK_Annot*)m_fxAnnotArray.GetAt(i);
+ if (pAnnot == focusedAnnot) {
+ KillFocusAnnot();
+ break;
+ }
+ }
+ }
+
CPDFDoc_Environment* pEnv = m_pSDKDoc->GetEnv();
- int nAnnotCount = m_fxAnnotArray.GetSize();
- for (int i = 0; i < nAnnotCount; i++) {
+ CPDFSDK_AnnotHandlerMgr* pAnnotHandlerMgr = pEnv->GetAnnotHandlerMgr();
+ ASSERT(pAnnotHandlerMgr);
+ for (int i = 0, count = m_fxAnnotArray.GetSize(); i < count; i++) {
CPDFSDK_Annot* pAnnot = (CPDFSDK_Annot*)m_fxAnnotArray.GetAt(i);
- // if there is a focused annot on the page, we should kill the focus first.
- if (pAnnot == m_pSDKDoc->GetFocusAnnot())
- KillFocusAnnot();
- CPDFSDK_AnnotHandlerMgr* pAnnotHandlerMgr = pEnv->GetAnnotHandlerMgr();
- ASSERT(pAnnotHandlerMgr);
pAnnotHandlerMgr->ReleaseAnnot(pAnnot);
}
m_fxAnnotArray.RemoveAll();
diff --git a/testing/embedder_test.cpp b/testing/embedder_test.cpp
index d62810d..151987f 100644
--- a/testing/embedder_test.cpp
+++ b/testing/embedder_test.cpp
@@ -232,6 +232,7 @@
formfillinfo->version = 1;
formfillinfo->FFI_SetTimer = SetTimerTrampoline;
formfillinfo->FFI_KillTimer = KillTimerTrampoline;
+ formfillinfo->FFI_GetPage = GetPageTrampoline;
formfillinfo->m_pJsPlatform = platform;
form_handle_ = FPDFDOC_InitFormFillEnvironment(document_, formfillinfo);
@@ -270,6 +271,15 @@
return page;
}
+FPDF_PAGE EmbedderTest::LoadAndCachePage(int page_number) {
+ FPDF_PAGE page = delegate_->GetPage(form_handle_, document_, page_number);
+ if (!page) {
+ return nullptr;
+ }
+ FORM_DoPageAAction(page, form_handle_, FPDFPAGE_AACTION_OPEN);
+ return page;
+}
+
FPDF_BITMAP EmbedderTest::RenderPage(FPDF_PAGE page) {
int width = static_cast<int>(FPDF_GetPageWidth(page));
int height = static_cast<int>(FPDF_GetPageHeight(page));
@@ -286,6 +296,22 @@
FPDF_ClosePage(page);
}
+FPDF_PAGE EmbedderTest::Delegate::GetPage(FPDF_FORMHANDLE form_handle,
+ FPDF_DOCUMENT document,
+ int page_index) {
+ auto it = m_pageMap.find(page_index);
+ if (it != m_pageMap.end()) {
+ return it->second;
+ }
+ FPDF_PAGE page = FPDF_LoadPage(document, page_index);
+ if (!page) {
+ return nullptr;
+ }
+ m_pageMap[page_index] = page;
+ FORM_OnAfterLoadPage(page, form_handle);
+ return page;
+}
+
// static
void EmbedderTest::UnsupportedHandlerTrampoline(UNSUPPORT_INFO* info,
int type) {
@@ -317,6 +343,14 @@
return test->delegate_->KillTimer(id);
}
+// static
+FPDF_PAGE EmbedderTest::GetPageTrampoline(FPDF_FORMFILLINFO* info,
+ FPDF_DOCUMENT document,
+ int page_index) {
+ EmbedderTest* test = static_cast<EmbedderTest*>(info);
+ return test->delegate_->GetPage(test->m_pFormfillinfo, document, page_index);
+}
+
// Can't use gtest-provided main since we need to stash the path to the
// executable in order to find the external V8 binary data files.
int main(int argc, char** argv) {
diff --git a/testing/embedder_test.h b/testing/embedder_test.h
index 4bd9d97..f490f82 100644
--- a/testing/embedder_test.h
+++ b/testing/embedder_test.h
@@ -5,6 +5,7 @@
#ifndef TESTING_EMBEDDER_TEST_H_
#define TESTING_EMBEDDER_TEST_H_
+#include <map>
#include <string>
#include "../public/fpdf_dataavail.h"
@@ -43,6 +44,14 @@
// Equivalent to FPDF_FORMFILLINFO::FFI_KillTimer().
virtual void KillTimer(int id) {}
+
+ // Equivalent to FPDF_FORMFILLINFO::FFI_GetPage().
+ virtual FPDF_PAGE GetPage(FPDF_FORMHANDLE form_handle,
+ FPDF_DOCUMENT document,
+ int page_index);
+
+ private:
+ std::map<int, FPDF_PAGE> m_pageMap;
};
EmbedderTest();
@@ -72,6 +81,10 @@
// Load a specific page of the open document.
virtual FPDF_PAGE LoadPage(int page_number);
+ // Load a specific page of the open document using delegate_->GetPage.
+ // delegate_->GetPage also caches loaded page.
+ virtual FPDF_PAGE LoadAndCachePage(int page_number);
+
// Convert a loaded page into a bitmap.
virtual FPDF_BITMAP RenderPage(FPDF_PAGE page);
@@ -106,6 +119,9 @@
int msecs,
TimerCallback fn);
static void KillTimerTrampoline(FPDF_FORMFILLINFO* info, int id);
+ static FPDF_PAGE GetPageTrampoline(FPDF_FORMFILLINFO* info,
+ FPDF_DOCUMENT document,
+ int page_index);
};
#endif // TESTING_EMBEDDER_TEST_H_
diff --git a/testing/resources/bug_507316.in b/testing/resources/bug_507316.in
new file mode 100644
index 0000000..cd3f81e
--- /dev/null
+++ b/testing/resources/bug_507316.in
@@ -0,0 +1,126 @@
+{{header}}
+{{object 1 0}} <<
+ /Type /Catalog
+ /Pages 2 0 R
+ /OpenAction 4 0 R
+ /AcroForm 3 0 R
+>>
+endobj
+{{object 2 0}} <<
+ /Type /Pages
+ /Count 3
+ /Kids [6 0 R 7 0 R 8 0 R ]
+>>
+endobj
+{{object 3 0}} <<
+ /CO [11 0 R]
+ /Fields [11 0 R 9 0 R]
+>>
+endobj
+{{object 4 0}} <<
+ /Type /Action
+ /S /JavaScript
+ /JS 5 0 R
+>>
+endobj
+{{object 5 0}} <<>>
+stream
+var i = 0;
+function run()
+{
+ t = this.getField('txtName1');
+ t2 = this.getField('txtName2');
+ t2.setFocus();
+ t.setFocus();
+ t.value='G';
+}
+
+function remove(){
+ if (i==1){
+ this.removeField('txtName2');
+ }
+ i++;
+}
+
+app.setTimeOut('run()',2000);
+endstream
+endobj
+{{object 6 0}} <<
+ /Type /Page
+ /Parent 2 0 R
+ /MediaBox [0 0 612 792]
+ /Resources <<>>
+>>
+endobj
+{{object 7 0}} <<
+ /Type /Page
+ /Parent 2 0 R
+ /MediaBox [0 0 612 792]
+ /Resources <<>>
+>>
+endobj
+{{object 8 0}} <<
+ /Type /Page
+ /Parent 2 0 R
+ /MediaBox [0 0 612 792]
+ /Resources <<>>
+ /Annots [9 0 R 11 0 R]
+>>
+endobj
+{{object 9 0}} <<
+ /FT /Tx
+ /Type /Annot
+ /Subtype /Widget
+ /T (txtName2)
+ /F 4
+ /AP <</N 10 0 R>>
+ /Rect [20 20 400 60]
+>>
+endobj
+{{object 10 0}} <<
+ /Type /XObject
+ /Subtype /Form
+ /FormType 1
+>>
+endobj
+{{object 11 0}} <<
+ /FT /Tx
+ /Type /Annot
+ /Subtype /Widget
+ /T (txtName1)
+ /F 4
+ /AP <</N 12 0 R>>
+ /Rect [200 200 400 260]
+ /AA 13 0 R
+>>
+endobj
+{{object 12 0}} <<
+ /Type /XObject
+ /Subtype /Form
+ /FormType 1
+>>
+endobj
+{{object 13 0}} <<
+ /C 14 0 R
+>>
+endobj
+{{object 14 0}} <<
+ /Type /Action
+ /S /JavaScript
+ /JS 15 0 R
+>>
+endobj
+{{object 15 0}} <<>>
+stream
+this.getField('txtName2').value='B';
+this.pageNum=1;
+remove();
+endstream
+endobj
+{{xref}}
+trailer <<
+ /Size 15
+ /Root 1 0 R
+>>
+{{startxref}}
+%%EOF
diff --git a/testing/resources/bug_507316.pdf b/testing/resources/bug_507316.pdf
new file mode 100644
index 0000000..13c7083
--- /dev/null
+++ b/testing/resources/bug_507316.pdf
@@ -0,0 +1,145 @@
+%PDF-1.7
+% ò¤ô
+1 0 obj <<
+ /Type /Catalog
+ /Pages 2 0 R
+ /OpenAction 4 0 R
+ /AcroForm 3 0 R
+>>
+endobj
+2 0 obj <<
+ /Type /Pages
+ /Count 3
+ /Kids [6 0 R 7 0 R 8 0 R ]
+>>
+endobj
+3 0 obj <<
+ /CO [11 0 R]
+ /Fields [11 0 R 9 0 R]
+>>
+endobj
+4 0 obj <<
+ /Type /Action
+ /S /JavaScript
+ /JS 5 0 R
+>>
+endobj
+5 0 obj <<>>
+stream
+var i = 0;
+function run()
+{
+ t = this.getField('txtName1');
+ t2 = this.getField('txtName2');
+ t2.setFocus();
+ t.setFocus();
+ t.value='G';
+}
+
+function remove(){
+ if (i==1){
+ this.removeField('txtName2');
+ }
+ i++;
+}
+
+app.setTimeOut('run()',2000);
+endstream
+endobj
+6 0 obj <<
+ /Type /Page
+ /Parent 2 0 R
+ /MediaBox [0 0 612 792]
+ /Resources <<>>
+>>
+endobj
+7 0 obj <<
+ /Type /Page
+ /Parent 2 0 R
+ /MediaBox [0 0 612 792]
+ /Resources <<>>
+>>
+endobj
+8 0 obj <<
+ /Type /Page
+ /Parent 2 0 R
+ /MediaBox [0 0 612 792]
+ /Resources <<>>
+ /Annots [9 0 R 11 0 R]
+>>
+endobj
+9 0 obj <<
+ /FT /Tx
+ /Type /Annot
+ /Subtype /Widget
+ /T (txtName2)
+ /F 4
+ /AP <</N 10 0 R>>
+ /Rect [20 20 400 60]
+>>
+endobj
+10 0 obj <<
+ /Type /XObject
+ /Subtype /Form
+ /FormType 1
+>>
+endobj
+11 0 obj <<
+ /FT /Tx
+ /Type /Annot
+ /Subtype /Widget
+ /T (txtName1)
+ /F 4
+ /AP <</N 12 0 R>>
+ /Rect [200 200 400 260]
+ /AA 13 0 R
+>>
+endobj
+12 0 obj <<
+ /Type /XObject
+ /Subtype /Form
+ /FormType 1
+>>
+endobj
+13 0 obj <<
+ /C 14 0 R
+>>
+endobj
+14 0 obj <<
+ /Type /Action
+ /S /JavaScript
+ /JS 15 0 R
+>>
+endobj
+15 0 obj <<>>
+stream
+this.getField('txtName2').value='B';
+this.pageNum=1;
+remove();
+endstream
+endobj
+xref
+0 16
+0000000000 65535 f
+0000000015 00000 n
+0000000106 00000 n
+0000000182 00000 n
+0000000243 00000 n
+0000000309 00000 n
+0000000602 00000 n
+0000000697 00000 n
+0000000792 00000 n
+0000000912 00000 n
+0000001043 00000 n
+0000001113 00000 n
+0000001261 00000 n
+0000001331 00000 n
+0000001365 00000 n
+0000001433 00000 n
+trailer <<
+ /Size 15
+ /Root 1 0 R
+>>
+startxref
+1534
+%%EOF
