Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(895)

Unified Diff: fpdfsdk/src/fpdfxfa/fpdfxfa_doc.cpp

Issue 1566903002: XFA: Change the destruction order inside CPDFXFA_Document to avoid UAFs. (Closed) Base URL: https://pdfium.googlesource.com/pdfium.git@xfa
Patch Set: actually flip the destruction order Created 4 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « fpdfsdk/include/fpdfxfa/fpdfxfa_doc.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: fpdfsdk/src/fpdfxfa/fpdfxfa_doc.cpp
diff --git a/fpdfsdk/src/fpdfxfa/fpdfxfa_doc.cpp b/fpdfsdk/src/fpdfxfa/fpdfxfa_doc.cpp
index 16f3209af3dac8357a6062eb2316924c69809891..06f1be654a804f51c9f79b97806d865568029125 100644
--- a/fpdfsdk/src/fpdfxfa/fpdfxfa_doc.cpp
+++ b/fpdfsdk/src/fpdfxfa/fpdfxfa_doc.cpp
@@ -45,14 +45,21 @@ CPDFXFA_Document::CPDFXFA_Document(CPDF_Document* pPDFDoc,
}
CPDFXFA_Document::~CPDFXFA_Document() {
+ if (m_pJSContext && m_pSDKDoc && m_pSDKDoc->GetEnv()) {
+ m_pSDKDoc->GetEnv()->GetJSRuntime()->ReleaseContext(m_pJSContext);
+ m_pJSContext = nullptr;
jun_fang 2016/01/07 04:30:47 Nit: Do we need to set m_pJSContext as nullptr in
Lei Zhang 2016/01/07 06:54:26 Done.
+ }
+
+ delete m_pSDKDoc;
+ m_pSDKDoc = nullptr;
jun_fang 2016/01/07 04:30:47 Nit: as above
Lei Zhang 2016/01/07 06:54:26 Done.
+
if (m_pPDFDoc) {
CPDF_Parser* pParser = (CPDF_Parser*)m_pPDFDoc->GetParser();
- if (pParser == NULL) {
- delete m_pPDFDoc;
- } else {
+ if (pParser)
delete pParser;
- }
- m_pPDFDoc = NULL;
+ else
+ delete m_pPDFDoc;
+ m_pPDFDoc = nullptr;
jun_fang 2016/01/07 04:30:47 nit: as above
Lei Zhang 2016/01/07 06:54:26 Done.
}
if (m_pXFADoc) {
IXFA_App* pApp = m_pApp->GetXFAApp();
@@ -63,17 +70,6 @@ CPDFXFA_Document::~CPDFXFA_Document() {
}
}
}
-
- if (m_pJSContext) {
- if (m_pSDKDoc && m_pSDKDoc->GetEnv()) {
- m_pSDKDoc->GetEnv()->GetJSRuntime()->ReleaseContext(m_pJSContext);
- m_pJSContext = NULL;
- }
- }
-
- if (m_pSDKDoc)
- delete m_pSDKDoc;
- m_pSDKDoc = NULL;
}
FX_BOOL CPDFXFA_Document::LoadXFADoc() {
@@ -204,13 +200,6 @@ CPDFSDK_Document* CPDFXFA_Document::GetSDKDocument(
return m_pSDKDoc;
}
-void CPDFXFA_Document::ReleaseSDKDoc() {
- if (m_pSDKDoc)
- delete m_pSDKDoc;
-
- m_pSDKDoc = NULL;
-}
-
void CPDFXFA_Document::FXRect2PDFRect(const CFX_RectF& fxRectF,
CPDF_Rect& pdfRect) {
pdfRect.left = fxRectF.left;
@@ -219,7 +208,6 @@ void CPDFXFA_Document::FXRect2PDFRect(const CFX_RectF& fxRectF,
pdfRect.bottom = fxRectF.top;
}
-//////////////////////////////////////////////////////////////////////////
void CPDFXFA_Document::SetChangeMark(IXFA_Doc* hDoc) {
if (hDoc == m_pXFADoc && m_pSDKDoc) {
m_pSDKDoc->SetChangeMark();
« no previous file with comments | « fpdfsdk/include/fpdfxfa/fpdfxfa_doc.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698