summaryrefslogtreecommitdiff
path: root/fpdfsdk/src
diff options
context:
space:
mode:
authorTom Sepez <tsepez@chromium.org>2015-09-09 09:58:10 -0700
committerTom Sepez <tsepez@chromium.org>2015-09-09 09:58:10 -0700
commit9241e5a43990859f6f9a94aaa2c488d0451039e3 (patch)
treed59fa133dccca79cb9b2e9da5930cae8aa6ad75e /fpdfsdk/src
parent343dbb841f4c12e819932e2b66dd70f817337d97 (diff)
downloadpdfium-9241e5a43990859f6f9a94aaa2c488d0451039e3.tar.xz
Fix heap use after free in CPDFSDK_Annot::GetPDFAnnot.
Use two seperate loops to kill current focus annot and to release annots in current page. Loop to kill current focus annot is run first, so it will not access deleted annots. BUG=507316 R=tsepez@chromium.org TEST=Reproduction steps mentioned in issue 507316 should not crash chrome. Unit test added to pdfium. Run pdfium_embeddertests.exe. Review URL: https://codereview.chromium.org/1312313006 .
Diffstat (limited to 'fpdfsdk/src')
-rw-r--r--fpdfsdk/src/fpdfformfill_embeddertest.cpp12
-rw-r--r--fpdfsdk/src/fsdk_mgr.cpp22
2 files changed, 26 insertions, 8 deletions
diff --git a/fpdfsdk/src/fpdfformfill_embeddertest.cpp b/fpdfsdk/src/fpdfformfill_embeddertest.cpp
index 6baad11531..56710b9f83 100644
--- a/fpdfsdk/src/fpdfformfill_embeddertest.cpp
+++ b/fpdfsdk/src/fpdfformfill_embeddertest.cpp
@@ -40,6 +40,18 @@ TEST_F(FPDFFormFillEmbeddertest, BUG_487928) {
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 3eca559285..9cbb9de4d9 100644
--- a/fpdfsdk/src/fsdk_mgr.cpp
+++ b/fpdfsdk/src/fsdk_mgr.cpp
@@ -634,16 +634,22 @@ CPDFSDK_PageView::CPDFSDK_PageView(CPDFSDK_Document* pSDKDoc, CPDF_Page* page)
}
CPDFSDK_PageView::~CPDFSDK_PageView() {
- CPDFDoc_Environment* pEnv = m_pSDKDoc->GetEnv();
- int nAnnotCount = m_fxAnnotArray.GetSize();
+ // 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;
+ }
+ }
+ }
- for (int i = 0; i < nAnnotCount; i++) {
+ CPDFDoc_Environment* pEnv = m_pSDKDoc->GetEnv();
+ 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();