Change the clear order of pDocPage and pDocRender

core/src/fpdfapi/fpdf_parser/fpdf_parser_document.cpp

 // Copyright 2014 PDFium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
  
 // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
 
 #include "../../../include/fpdfapi/fpdf_parser.h"
 #include "../../../include/fpdfapi/fpdf_module.h"
 extern FX_LPVOID PDFPreviewInitCache(CPDF_Document* pDoc);
 extern void PDFPreviewClearCache(FX_LPVOID pCache);
@@ skipping 77 matching lines @@
         m_dwFirstPageObjNum = pObjNum->GetInteger();
     }
 }
 void CPDF_Document::LoadPages()
 {
     m_PageList.SetSize(_GetPageCount());
 }
 extern void FPDF_TTFaceMapper_ReleaseDoc(CPDF_Document*);
 CPDF_Document::~CPDF_Document()
 {
-    if (m_pDocRender) {
-        CPDF_ModuleMgr::Get()->GetRenderModule()->DestroyDocData(m_pDocRender);
-    }
     if (m_pDocPage) {
         CPDF_ModuleMgr::Get()->GetPageModule()->ReleaseDoc(this);
         CPDF_ModuleMgr::Get()->GetPageModule()->ClearStockFont(this);
     }
+    if (m_pDocRender) {

[Tom Sepez, 2014/10/21 21:05:29]

Are you sure this doesn't introduce new UAFs from render objects that are now
outliving the page objects?  Is there a design principle somewhere that states
one must outlast the other? Without knowing the details, it feels like a design
error if render objects aren't shorter lived than the pages they represent.

[Bo Xu, 2014/10/21 21:19:42]

Hi Tom, I have the same question and checked the devs in Chinese team before. So
DestroyDocData(m_pDocRender)will calls into CPDF_DocRenderData::Clear(FX_BOOL
bRelease) and it will release m_Type3FaceMap, m_TransferFuncMap and
m_pFontCache. This is very similar to CPDF_DocPageData::Clear(FX_BOOL
bForceRelease) where all those page objects are released.

So what I got from them are m_pDocPage and m_pDocRender are kinda independent.
m_pDocRender is more light-weighted (only 3 resources) and are not dependent on
m_pDocPage. I checked the code and that seems to be the case.

+        CPDF_ModuleMgr::Get()->GetRenderModule()->DestroyDocData(m_pDocRender);
+    }
 }
 #define         FX_MAX_PAGE_LEVEL                       1024
 CPDF_Dictionary* CPDF_Document::_FindPDFPage(CPDF_Dictionary* pPages, int iPage, int nPagesToGo, int level)
 {
     CPDF_Array* pKidList = pPages->GetArray(FX_BSTRC("Kids"));
     if (pKidList == NULL) {
         if (nPagesToGo == 0) {
             return pPages;
         }
         return NULL;
@@ skipping 279 matching lines @@
     if (m_pDocPage) {
         CPDF_ModuleMgr::Get()->GetPageModule()->ClearDoc(this);
     }
 }
 void CPDF_Document::ClearRenderData()
 {
     if (m_pDocRender) {
         CPDF_ModuleMgr::Get()->GetRenderModule()->ClearDocData(m_pDocRender);
     }
 }