Fix memory leaks involving InsertIndirectObject()

BUG=447331
R=tsepez@chromium.org

Review URL: https://codereview.chromium.org/1514093002 .

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<CPDF_Object*>(value);
-      if (pObj->GetGenNum() <= pValue->GetGenNum())
-        return;
-      pValue->Destroy();
+      CPDF_Object* pExistingObj = static_cast<CPDF_Object*>(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();
     }
   }