diff options
Diffstat (limited to 'core/fpdfapi/fpdf_parser')
-rw-r--r-- | core/fpdfapi/fpdf_parser/cpdf_array.cpp | 2 | ||||
-rw-r--r-- | core/fpdfapi/fpdf_parser/cpdf_dictionary.cpp | 2 | ||||
-rw-r--r-- | core/fpdfapi/fpdf_parser/cpdf_object.cpp | 4 | ||||
-rw-r--r-- | core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp | 32 | ||||
-rw-r--r-- | core/fpdfapi/fpdf_parser/cpdf_parser.cpp | 37 | ||||
-rw-r--r-- | core/fpdfapi/fpdf_parser/cpdf_stream.cpp | 6 |
6 files changed, 47 insertions, 36 deletions
diff --git a/core/fpdfapi/fpdf_parser/cpdf_array.cpp b/core/fpdfapi/fpdf_parser/cpdf_array.cpp index 26e65bca34..a3be55b9f0 100644 --- a/core/fpdfapi/fpdf_parser/cpdf_array.cpp +++ b/core/fpdfapi/fpdf_parser/cpdf_array.cpp @@ -23,7 +23,7 @@ CPDF_Array::~CPDF_Array() { // in case of cyclic references. m_ObjNum = kInvalidObjNum; for (auto& it : m_Objects) { - if (it) + if (it && it->GetObjNum() != kInvalidObjNum) it->Release(); } } diff --git a/core/fpdfapi/fpdf_parser/cpdf_dictionary.cpp b/core/fpdfapi/fpdf_parser/cpdf_dictionary.cpp index 5696fc0c8d..eb40de7d49 100644 --- a/core/fpdfapi/fpdf_parser/cpdf_dictionary.cpp +++ b/core/fpdfapi/fpdf_parser/cpdf_dictionary.cpp @@ -30,7 +30,7 @@ CPDF_Dictionary::~CPDF_Dictionary() { // in case of cyclic references. m_ObjNum = kInvalidObjNum; for (const auto& it : m_Map) { - if (it.second) + if (it.second && it.second->GetObjNum() != kInvalidObjNum) it.second->Release(); } } diff --git a/core/fpdfapi/fpdf_parser/cpdf_object.cpp b/core/fpdfapi/fpdf_parser/cpdf_object.cpp index ba7490a13f..d74003112f 100644 --- a/core/fpdfapi/fpdf_parser/cpdf_object.cpp +++ b/core/fpdfapi/fpdf_parser/cpdf_object.cpp @@ -38,9 +38,7 @@ CPDF_Object* CPDF_Object::CloneNonCyclic( } void CPDF_Object::Release() { - if (m_ObjNum) - return; - + CHECK(!m_ObjNum); delete this; } diff --git a/core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp b/core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp index 9b702099bb..6181571229 100644 --- a/core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp +++ b/core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp @@ -25,6 +25,7 @@ namespace { using ScopedArray = std::unique_ptr<CPDF_Array, ReleaseDeleter<CPDF_Array>>; using ScopedDict = std::unique_ptr<CPDF_Dictionary, ReleaseDeleter<CPDF_Dictionary>>; +using ScopedStream = std::unique_ptr<CPDF_Stream, ReleaseDeleter<CPDF_Stream>>; void TestArrayAccessors(const CPDF_Array* arr, size_t index, @@ -95,8 +96,10 @@ class PDFObjectsTest : public testing::Test { // Indirect references to indirect objects. m_ObjHolder.reset(new CPDF_IndirectObjectHolder()); - m_IndirectObjs = {boolean_true_obj, number_int_obj, str_spec_obj, name_obj, - m_ArrayObj, m_DictObj, stream_obj}; + m_IndirectObjs = {boolean_true_obj->Clone(), number_int_obj->Clone(), + str_spec_obj->Clone(), name_obj->Clone(), + m_ArrayObj->Clone(), m_DictObj->Clone(), + stream_obj->Clone()}; for (size_t i = 0; i < m_IndirectObjs.size(); ++i) { m_ObjHolder->AddIndirectObject(m_IndirectObjs[i]); m_RefObjs.emplace_back(new CPDF_Reference( @@ -104,7 +107,7 @@ class PDFObjectsTest : public testing::Test { } } - bool Equal(CPDF_Object* obj1, CPDF_Object* obj2) { + bool Equal(const CPDF_Object* obj1, const CPDF_Object* obj2) { if (obj1 == obj2) return true; if (!obj1 || !obj2 || obj1->GetType() != obj2->GetType()) @@ -253,7 +256,7 @@ TEST_F(PDFObjectsTest, GetDict) { const CPDF_Dictionary* const indirect_obj_results[] = { nullptr, nullptr, nullptr, nullptr, nullptr, m_DictObj, m_StreamDictObj}; for (size_t i = 0; i < m_RefObjs.size(); ++i) - EXPECT_EQ(indirect_obj_results[i], m_RefObjs[i]->GetDict()); + EXPECT_TRUE(Equal(indirect_obj_results[i], m_RefObjs[i]->GetDict())); } TEST_F(PDFObjectsTest, GetArray) { @@ -787,10 +790,9 @@ TEST(PDFDictionaryTest, CloneDirectObject) { TEST(PDFObjectTest, CloneCheckLoop) { { - // Create an object with a reference loop. - ScopedArray arr_obj(new CPDF_Array); - // Dictionary object. + // Create a dictionary/array pair with a reference loop. CPDF_Dictionary* dict_obj = new CPDF_Dictionary(); + ScopedArray arr_obj(new CPDF_Array); dict_obj->SetFor("arr", arr_obj.get()); arr_obj->InsertAt(0, dict_obj); @@ -806,6 +808,22 @@ TEST(PDFObjectTest, CloneCheckLoop) { EXPECT_EQ(nullptr, cloned_dict->AsDictionary()->GetObjectFor("arr")); } { + // Create a dictionary/stream pair with a reference loop. + CPDF_Dictionary* dict_obj = new CPDF_Dictionary(); + ScopedStream stream_obj(new CPDF_Stream(nullptr, 0, dict_obj)); + dict_obj->SetFor("stream", stream_obj.get()); + + // Clone this object to see whether stack overflow will be triggered. + ScopedStream cloned_stream(stream_obj->Clone()->AsStream()); + // Cloned object should be the same as the original. + ASSERT_TRUE(cloned_stream); + CPDF_Object* cloned_dict = cloned_stream->GetDict(); + ASSERT_TRUE(cloned_dict); + ASSERT_TRUE(cloned_dict->IsDictionary()); + // Recursively referenced object is not cloned. + EXPECT_EQ(nullptr, cloned_dict->AsDictionary()->GetObjectFor("stream")); + } + { CPDF_IndirectObjectHolder objects_holder; // Create an object with a reference loop. CPDF_Dictionary* dict_obj = new CPDF_Dictionary(); diff --git a/core/fpdfapi/fpdf_parser/cpdf_parser.cpp b/core/fpdfapi/fpdf_parser/cpdf_parser.cpp index 182d3869bc..bc3dfe6021 100644 --- a/core/fpdfapi/fpdf_parser/cpdf_parser.cpp +++ b/core/fpdfapi/fpdf_parser/cpdf_parser.cpp @@ -956,36 +956,34 @@ FX_BOOL CPDF_Parser::RebuildCrossRef() { } FX_BOOL CPDF_Parser::LoadCrossRefV5(FX_FILESIZE* pos, FX_BOOL bMainXRef) { - CPDF_Object* pObject = ParseIndirectObjectAt(m_pDocument, *pos, 0); + std::unique_ptr<CPDF_Object> pObject( + ParseIndirectObjectAt(m_pDocument, *pos, 0)); if (!pObject) return FALSE; + CPDF_Object* pUnownedObject = pObject.get(); + if (m_pDocument) { CPDF_Dictionary* pRootDict = m_pDocument->GetRoot(); - if (pRootDict && pRootDict->GetObjNum() == pObject->m_ObjNum) { - // If |pObject| has an objnum assigned then this will leak as Release() - // will early exit. - if (pObject->IsStream()) - pObject->Release(); + if (pRootDict && pRootDict->GetObjNum() == pObject->m_ObjNum) return FALSE; - } - if (!m_pDocument->ReplaceIndirectObjectIfHigherGeneration(pObject->m_ObjNum, - pObject)) { + // Takes ownership of object (std::move someday). + uint32_t objnum = pObject->m_ObjNum; + if (!m_pDocument->ReplaceIndirectObjectIfHigherGeneration( + objnum, pObject.release())) { return FALSE; } } - CPDF_Stream* pStream = pObject->AsStream(); + CPDF_Stream* pStream = pUnownedObject->AsStream(); if (!pStream) return FALSE; CPDF_Dictionary* pDict = pStream->GetDict(); *pos = pDict->GetIntegerFor("Prev"); int32_t size = pDict->GetIntegerFor("Size"); - if (size < 0) { - pStream->Release(); + if (size < 0) return FALSE; - } CPDF_Dictionary* pNewTrailer = ToDictionary(pDict->Clone()); if (bMainXRef) { @@ -1017,10 +1015,8 @@ FX_BOOL CPDF_Parser::LoadCrossRefV5(FX_FILESIZE* pos, FX_BOOL bMainXRef) { arrIndex.push_back(std::make_pair(0, size)); pArray = pDict->GetArrayFor("W"); - if (!pArray) { - pStream->Release(); + if (!pArray) return FALSE; - } CFX_ArrayTemplate<uint32_t> WidthArray; FX_SAFE_UINT32 dwAccWidth = 0; @@ -1029,10 +1025,8 @@ FX_BOOL CPDF_Parser::LoadCrossRefV5(FX_FILESIZE* pos, FX_BOOL bMainXRef) { dwAccWidth += WidthArray[i]; } - if (!dwAccWidth.IsValid() || WidthArray.GetSize() < 3) { - pStream->Release(); + if (!dwAccWidth.IsValid() || WidthArray.GetSize() < 3) return FALSE; - } uint32_t totalWidth = dwAccWidth.ValueOrDie(); CPDF_StreamAcc acc; @@ -1092,17 +1086,14 @@ FX_BOOL CPDF_Parser::LoadCrossRefV5(FX_FILESIZE* pos, FX_BOOL bMainXRef) { if (type == 1) { m_SortedOffset.insert(offset); } else { - if (offset < 0 || !IsValidObjectNumber(offset)) { - pStream->Release(); + if (offset < 0 || !IsValidObjectNumber(offset)) return FALSE; - } m_ObjectInfo[offset].type = 255; } } } segindex += count; } - pStream->Release(); return TRUE; } diff --git a/core/fpdfapi/fpdf_parser/cpdf_stream.cpp b/core/fpdfapi/fpdf_parser/cpdf_stream.cpp index 4f6d046397..6a0bf05937 100644 --- a/core/fpdfapi/fpdf_parser/cpdf_stream.cpp +++ b/core/fpdfapi/fpdf_parser/cpdf_stream.cpp @@ -19,7 +19,11 @@ CPDF_Stream::CPDF_Stream(uint8_t* pData, uint32_t size, CPDF_Dictionary* pDict) m_dwSize(size), m_pDataBuf(pData) {} -CPDF_Stream::~CPDF_Stream() {} +CPDF_Stream::~CPDF_Stream() { + m_ObjNum = kInvalidObjNum; + if (m_pDict && m_pDict->GetObjNum() == kInvalidObjNum) + m_pDict.release(); // lowercase release, release ownership. +} CPDF_Object::Type CPDF_Stream::GetType() const { return STREAM; |