From e0bb17b98ce909edee8b14c2b2036b71b3f75593 Mon Sep 17 00:00:00 2001 From: Jun Fang Date: Fri, 11 Dec 2015 22:01:55 -0800 Subject: Merge to XFA: Fix memory leaks involving InsertIndirectObject() BUG=447331 R=tsepez@chromium.org Review URL: https://codereview.chromium.org/1514093002 . Review URL: https://codereview.chromium.org/1515403003 . --- core/include/fpdfapi/fpdf_objects.h | 4 ++-- .../fpdfapi/fpdf_parser/fpdf_parser_objects.cpp | 25 +++++++++++----------- .../src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp | 10 +++++---- 3 files changed, 21 insertions(+), 18 deletions(-) diff --git a/core/include/fpdfapi/fpdf_objects.h b/core/include/fpdfapi/fpdf_objects.h index 8522e9be64..c220a0cb1e 100644 --- a/core/include/fpdfapi/fpdf_objects.h +++ b/core/include/fpdfapi/fpdf_objects.h @@ -611,8 +611,8 @@ class CPDF_IndirectObjects { FX_DWORD AddIndirectObject(CPDF_Object* pObj); void ReleaseIndirectObject(FX_DWORD objnum); - - void InsertIndirectObject(FX_DWORD objnum, CPDF_Object* pObj); + // Takes ownership of |pObj|. + FX_BOOL InsertIndirectObject(FX_DWORD objnum, CPDF_Object* pObj); FX_DWORD GetLastObjNum() const; diff --git a/core/src/fpdfapi/fpdf_parser/fpdf_parser_objects.cpp b/core/src/fpdfapi/fpdf_parser/fpdf_parser_objects.cpp index ad4038d71b..fe20a92a5b 100644 --- a/core/src/fpdfapi/fpdf_parser/fpdf_parser_objects.cpp +++ b/core/src/fpdfapi/fpdf_parser/fpdf_parser_objects.cpp @@ -1156,25 +1156,26 @@ void CPDF_IndirectObjects::ReleaseIndirectObject(FX_DWORD objnum) { pValue->Destroy(); m_IndirectObjs.RemoveKey((void*)(uintptr_t)objnum); } -void CPDF_IndirectObjects::InsertIndirectObject(FX_DWORD objnum, - CPDF_Object* pObj) { - if (objnum == 0 || pObj == NULL) { - return; - } - void* value = NULL; +FX_BOOL CPDF_IndirectObjects::InsertIndirectObject(FX_DWORD objnum, + CPDF_Object* pObj) { + if (!objnum || !pObj) + return FALSE; + void* value = nullptr; if (m_IndirectObjs.Lookup((void*)(uintptr_t)objnum, value)) { if (value) { - CPDF_Object* pValue = static_cast(value); - if (pObj->GetGenNum() <= pValue->GetGenNum()) - return; - pValue->Destroy(); + CPDF_Object* pExistingObj = static_cast(value); + if (pObj->GetGenNum() <= pExistingObj->GetGenNum()) { + pObj->Destroy(); + return FALSE; + } + pExistingObj->Destroy(); } } pObj->m_ObjNum = objnum; m_IndirectObjs.SetAt((void*)(uintptr_t)objnum, pObj); - if (m_LastObjNum < objnum) { + if (m_LastObjNum < objnum) m_LastObjNum = objnum; - } + return TRUE; } FX_DWORD CPDF_IndirectObjects::GetLastObjNum() const { return m_LastObjNum; diff --git a/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp b/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp index ca3a5f67a2..61d9749c8e 100644 --- a/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp +++ b/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp @@ -987,16 +987,17 @@ FX_BOOL CPDF_Parser::LoadCrossRefV5(FX_FILESIZE* pos, FX_BOOL bMainXRef) { CPDF_Object* pObject = ParseIndirectObjectAt(m_pDocument, *pos, 0, nullptr); if (!pObject) return FALSE; - if (m_pDocument) { + FX_BOOL bInserted = FALSE; CPDF_Dictionary* pDict = m_pDocument->GetRoot(); if (!pDict || pDict->GetObjNum() != pObject->m_ObjNum) { - m_pDocument->InsertIndirectObject(pObject->m_ObjNum, pObject); + bInserted = m_pDocument->InsertIndirectObject(pObject->m_ObjNum, pObject); } else { if (pObject->IsStream()) pObject->Release(); - return FALSE; } + if (!bInserted) + return FALSE; } CPDF_Stream* pStream = pObject->AsStream(); @@ -4553,7 +4554,8 @@ CPDF_Dictionary* CPDF_DataAvail::GetPage(int index) { if (!pPageDict) { return nullptr; } - m_pDocument->InsertIndirectObject(dwObjNum, pPageDict); + if (!m_pDocument->InsertIndirectObject(dwObjNum, pPageDict)) + return nullptr; return pPageDict->GetDict(); } } -- cgit v1.2.3