From a470b5e5371d0674d06068ec38d0d3c3279e85e1 Mon Sep 17 00:00:00 2001 From: weili Date: Tue, 23 Aug 2016 22:08:37 -0700 Subject: Fix stack overflow in object Clone() functions For some complex objects such as CPDF_Dictionary, CPDF_Array, CPDF_Stream, and CPDF_Reference, Clone() could be executed with infinite recursion to cause the stack overflow. Fix this by checking already cloned objects to avoid recursion. BUG=pdfium:513 Review-Url: https://codereview.chromium.org/2250533002 --- core/fpdfapi/fpdf_page/cpdf_image.cpp | 2 +- core/fpdfapi/fpdf_parser/cpdf_array.cpp | 16 ++++++- core/fpdfapi/fpdf_parser/cpdf_array_unittest.cpp | 4 +- core/fpdfapi/fpdf_parser/cpdf_boolean.cpp | 2 +- core/fpdfapi/fpdf_parser/cpdf_boolean.h | 2 +- core/fpdfapi/fpdf_parser/cpdf_dictionary.cpp | 19 ++++++-- core/fpdfapi/fpdf_parser/cpdf_name.cpp | 2 +- core/fpdfapi/fpdf_parser/cpdf_null.cpp | 2 +- core/fpdfapi/fpdf_parser/cpdf_null.h | 2 +- core/fpdfapi/fpdf_parser/cpdf_number.cpp | 2 +- core/fpdfapi/fpdf_parser/cpdf_object.cpp | 15 +++++++ core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp | 51 ++++++++++++++++++++++ core/fpdfapi/fpdf_parser/cpdf_reference.cpp | 14 +++++- core/fpdfapi/fpdf_parser/cpdf_stream.cpp | 17 ++++++-- core/fpdfapi/fpdf_parser/cpdf_string.cpp | 2 +- core/fpdfapi/fpdf_parser/include/cpdf_array.h | 7 ++- core/fpdfapi/fpdf_parser/include/cpdf_dictionary.h | 7 ++- core/fpdfapi/fpdf_parser/include/cpdf_name.h | 2 +- core/fpdfapi/fpdf_parser/include/cpdf_number.h | 2 +- core/fpdfapi/fpdf_parser/include/cpdf_object.h | 31 +++++++++++-- core/fpdfapi/fpdf_parser/include/cpdf_reference.h | 7 ++- core/fpdfapi/fpdf_parser/include/cpdf_stream.h | 7 ++- core/fpdfapi/fpdf_parser/include/cpdf_string.h | 2 +- core/fpdfdoc/cpdf_interform.cpp | 8 ++-- fpdfsdk/fpdfppo.cpp | 2 +- 25 files changed, 191 insertions(+), 36 deletions(-) diff --git a/core/fpdfapi/fpdf_page/cpdf_image.cpp b/core/fpdfapi/fpdf_page/cpdf_image.cpp index af361eb9f6..2b3625bfc2 100644 --- a/core/fpdfapi/fpdf_page/cpdf_image.cpp +++ b/core/fpdfapi/fpdf_page/cpdf_image.cpp @@ -69,7 +69,7 @@ CPDF_Image* CPDF_Image::Clone() { CPDF_Image* pImage = new CPDF_Image(m_pDocument, ToStream(m_pStream->Clone()), m_bInline); if (m_bInline) - pImage->SetInlineDict(ToDictionary(m_pInlineDict->Clone(TRUE))); + pImage->SetInlineDict(ToDictionary(m_pInlineDict->CloneDirectObject())); return pImage; } diff --git a/core/fpdfapi/fpdf_parser/cpdf_array.cpp b/core/fpdfapi/fpdf_parser/cpdf_array.cpp index a047b3af7b..83f99c215b 100644 --- a/core/fpdfapi/fpdf_parser/cpdf_array.cpp +++ b/core/fpdfapi/fpdf_parser/cpdf_array.cpp @@ -11,10 +11,14 @@ #include "core/fpdfapi/fpdf_parser/include/cpdf_reference.h" #include "core/fpdfapi/fpdf_parser/include/cpdf_stream.h" #include "core/fpdfapi/fpdf_parser/include/cpdf_string.h" +#include "third_party/base/stl_util.h" CPDF_Array::CPDF_Array() {} CPDF_Array::~CPDF_Array() { + // Mark the object as deleted so that it will not be deleted again + // in case of cyclic references. + m_ObjNum = kInvalidObjNum; for (auto& it : m_Objects) { if (it) it->Release(); @@ -37,11 +41,19 @@ const CPDF_Array* CPDF_Array::AsArray() const { return this; } -CPDF_Object* CPDF_Array::Clone(FX_BOOL bDirect) const { +CPDF_Object* CPDF_Array::Clone() const { + return CloneObjectNonCyclic(false); +} + +CPDF_Object* CPDF_Array::CloneNonCyclic( + bool bDirect, + std::set* pVisited) const { + pVisited->insert(this); CPDF_Array* pCopy = new CPDF_Array(); for (size_t i = 0; i < GetCount(); i++) { CPDF_Object* value = m_Objects.at(i); - pCopy->m_Objects.push_back(value->Clone(bDirect)); + if (!pdfium::ContainsKey(*pVisited, value)) + pCopy->m_Objects.push_back(value->CloneNonCyclic(bDirect, pVisited)); } return pCopy; } diff --git a/core/fpdfapi/fpdf_parser/cpdf_array_unittest.cpp b/core/fpdfapi/fpdf_parser/cpdf_array_unittest.cpp index efad3c38f6..41a49cadab 100644 --- a/core/fpdfapi/fpdf_parser/cpdf_array_unittest.cpp +++ b/core/fpdfapi/fpdf_parser/cpdf_array_unittest.cpp @@ -125,10 +125,10 @@ TEST(cpdf_array, Clone) { ASSERT_EQ(kNumOfRows, arr->GetCount()); // Not dereferencing reference objects means just creating new references // instead of new copies of direct objects. - ScopedArray arr1(arr->Clone(FALSE)->AsArray()); + ScopedArray arr1(arr->Clone()->AsArray()); EXPECT_EQ(arr->GetCount(), arr1->GetCount()); // Dereferencing reference objects creates new copies of direct objects. - ScopedArray arr2(arr->Clone(TRUE)->AsArray()); + ScopedArray arr2(arr->CloneDirectObject()->AsArray()); EXPECT_EQ(arr->GetCount(), arr2->GetCount()); for (size_t i = 0; i < kNumOfRows; ++i) { CPDF_Array* arr_elem = arr->GetObjectAt(i)->AsArray(); diff --git a/core/fpdfapi/fpdf_parser/cpdf_boolean.cpp b/core/fpdfapi/fpdf_parser/cpdf_boolean.cpp index c5fd277d74..be0b7e99a3 100644 --- a/core/fpdfapi/fpdf_parser/cpdf_boolean.cpp +++ b/core/fpdfapi/fpdf_parser/cpdf_boolean.cpp @@ -16,7 +16,7 @@ CPDF_Object::Type CPDF_Boolean::GetType() const { return BOOLEAN; } -CPDF_Object* CPDF_Boolean::Clone(FX_BOOL bDirect) const { +CPDF_Object* CPDF_Boolean::Clone() const { return new CPDF_Boolean(m_bValue); } diff --git a/core/fpdfapi/fpdf_parser/cpdf_boolean.h b/core/fpdfapi/fpdf_parser/cpdf_boolean.h index adbb721f35..231b8fe9a4 100644 --- a/core/fpdfapi/fpdf_parser/cpdf_boolean.h +++ b/core/fpdfapi/fpdf_parser/cpdf_boolean.h @@ -18,7 +18,7 @@ class CPDF_Boolean : public CPDF_Object { // CPDF_Object. Type GetType() const override; - CPDF_Object* Clone(FX_BOOL bDirect = FALSE) const override; + CPDF_Object* Clone() const override; CFX_ByteString GetString() const override; int GetInteger() const override; void SetString(const CFX_ByteString& str) override; diff --git a/core/fpdfapi/fpdf_parser/cpdf_dictionary.cpp b/core/fpdfapi/fpdf_parser/cpdf_dictionary.cpp index 78b88a1b2f..8fef074d4b 100644 --- a/core/fpdfapi/fpdf_parser/cpdf_dictionary.cpp +++ b/core/fpdfapi/fpdf_parser/cpdf_dictionary.cpp @@ -18,6 +18,7 @@ CPDF_Dictionary::CPDF_Dictionary() {} CPDF_Dictionary::~CPDF_Dictionary() { + m_ObjNum = kInvalidObjNum; for (const auto& it : m_Map) it.second->Release(); } @@ -44,10 +45,22 @@ const CPDF_Dictionary* CPDF_Dictionary::AsDictionary() const { return this; } -CPDF_Object* CPDF_Dictionary::Clone(FX_BOOL bDirect) const { +CPDF_Object* CPDF_Dictionary::Clone() const { + return CloneObjectNonCyclic(false); +} + +CPDF_Object* CPDF_Dictionary::CloneNonCyclic( + bool bDirect, + std::set* pVisited) const { + pVisited->insert(this); CPDF_Dictionary* pCopy = new CPDF_Dictionary(); - for (const auto& it : *this) - pCopy->m_Map.insert(std::make_pair(it.first, it.second->Clone(bDirect))); + for (const auto& it : *this) { + CPDF_Object* value = it.second; + if (!pdfium::ContainsKey(*pVisited, value)) { + pCopy->m_Map.insert( + std::make_pair(it.first, value->CloneNonCyclic(bDirect, pVisited))); + } + } return pCopy; } diff --git a/core/fpdfapi/fpdf_parser/cpdf_name.cpp b/core/fpdfapi/fpdf_parser/cpdf_name.cpp index 7d67b51d7c..004dd54392 100644 --- a/core/fpdfapi/fpdf_parser/cpdf_name.cpp +++ b/core/fpdfapi/fpdf_parser/cpdf_name.cpp @@ -16,7 +16,7 @@ CPDF_Object::Type CPDF_Name::GetType() const { return NAME; } -CPDF_Object* CPDF_Name::Clone(FX_BOOL bDirect) const { +CPDF_Object* CPDF_Name::Clone() const { return new CPDF_Name(m_Name); } diff --git a/core/fpdfapi/fpdf_parser/cpdf_null.cpp b/core/fpdfapi/fpdf_parser/cpdf_null.cpp index c74daf300f..2149eaff62 100644 --- a/core/fpdfapi/fpdf_parser/cpdf_null.cpp +++ b/core/fpdfapi/fpdf_parser/cpdf_null.cpp @@ -12,6 +12,6 @@ CPDF_Object::Type CPDF_Null::GetType() const { return NULLOBJ; } -CPDF_Object* CPDF_Null::Clone(FX_BOOL bDirect) const { +CPDF_Object* CPDF_Null::Clone() const { return new CPDF_Null; } diff --git a/core/fpdfapi/fpdf_parser/cpdf_null.h b/core/fpdfapi/fpdf_parser/cpdf_null.h index ae33bb0256..9ffb0b13c1 100644 --- a/core/fpdfapi/fpdf_parser/cpdf_null.h +++ b/core/fpdfapi/fpdf_parser/cpdf_null.h @@ -15,7 +15,7 @@ class CPDF_Null : public CPDF_Object { // CPDF_Object. Type GetType() const override; - CPDF_Object* Clone(FX_BOOL bDirect = FALSE) const override; + CPDF_Object* Clone() const override; }; #endif // CORE_FPDFAPI_FPDF_PARSER_CPDF_NULL_H_ diff --git a/core/fpdfapi/fpdf_parser/cpdf_number.cpp b/core/fpdfapi/fpdf_parser/cpdf_number.cpp index 5012e308ce..9406cc9f11 100644 --- a/core/fpdfapi/fpdf_parser/cpdf_number.cpp +++ b/core/fpdfapi/fpdf_parser/cpdf_number.cpp @@ -21,7 +21,7 @@ CPDF_Object::Type CPDF_Number::GetType() const { return NUMBER; } -CPDF_Object* CPDF_Number::Clone(FX_BOOL bDirect) const { +CPDF_Object* CPDF_Number::Clone() const { return m_bInteger ? new CPDF_Number(m_Integer) : new CPDF_Number(m_Float); } diff --git a/core/fpdfapi/fpdf_parser/cpdf_object.cpp b/core/fpdfapi/fpdf_parser/cpdf_object.cpp index ec967d1032..7da12a2b4f 100644 --- a/core/fpdfapi/fpdf_parser/cpdf_object.cpp +++ b/core/fpdfapi/fpdf_parser/cpdf_object.cpp @@ -22,6 +22,21 @@ CPDF_Object* CPDF_Object::GetDirect() const { return const_cast(this); } +CPDF_Object* CPDF_Object::CloneObjectNonCyclic(bool bDirect) const { + std::set visited_objs; + return CloneNonCyclic(bDirect, &visited_objs); +} + +CPDF_Object* CPDF_Object::CloneDirectObject() const { + return CloneObjectNonCyclic(true); +} + +CPDF_Object* CPDF_Object::CloneNonCyclic( + bool bDirect, + std::set* pVisited) const { + return Clone(); +} + void CPDF_Object::Release() { if (m_ObjNum) return; diff --git a/core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp b/core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp index 52b01fe3b4..018300d462 100644 --- a/core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp +++ b/core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp @@ -23,6 +23,8 @@ namespace { using ScopedArray = std::unique_ptr>; +using ScopedDict = + std::unique_ptr>; void TestArrayAccessors(const CPDF_Array* arr, size_t index, @@ -744,3 +746,52 @@ TEST(PDFArrayTest, AddReferenceAndGetObjectAt) { EXPECT_EQ(indirect_objs[i], arr1->GetDirectObjectAt(i)); } } + +TEST(PDFObjectTest, CloneCheckLoop) { + { + // Create an object with a reference loop. + ScopedArray arr_obj(new CPDF_Array); + // Dictionary object. + CPDF_Dictionary* dict_obj = new CPDF_Dictionary; + dict_obj->SetAt("arr", arr_obj.get()); + arr_obj->InsertAt(0, dict_obj); + + // Clone this object to see whether stack overflow will be triggered. + ScopedArray cloned_array(arr_obj->Clone()->AsArray()); + // Cloned object should be the same as the original. + ASSERT_TRUE(cloned_array); + EXPECT_EQ(1u, cloned_array->GetCount()); + CPDF_Object* cloned_dict = cloned_array->GetObjectAt(0); + ASSERT_TRUE(cloned_dict); + ASSERT_TRUE(cloned_dict->IsDictionary()); + // Recursively referenced object is not cloned. + EXPECT_EQ(nullptr, cloned_dict->AsDictionary()->GetObjectBy("arr")); + } + { + std::unique_ptr m_ObjHolder( + new CPDF_IndirectObjectHolder(nullptr)); + // Create an object with a reference loop. + CPDF_Dictionary* dict_obj = new CPDF_Dictionary; + CPDF_Array* arr_obj = new CPDF_Array; + m_ObjHolder->AddIndirectObject(dict_obj); + EXPECT_EQ(1u, dict_obj->GetObjNum()); + dict_obj->SetAt("arr", arr_obj); + arr_obj->InsertAt(0, dict_obj, m_ObjHolder.get()); + CPDF_Object* elem0 = arr_obj->GetObjectAt(0); + ASSERT_TRUE(elem0); + ASSERT_TRUE(elem0->IsReference()); + EXPECT_EQ(1u, elem0->AsReference()->GetRefObjNum()); + EXPECT_EQ(dict_obj, elem0->AsReference()->GetDirect()); + + // Clone this object to see whether stack overflow will be triggered. + ScopedDict cloned_dict(ToDictionary(dict_obj->CloneDirectObject())); + // Cloned object should be the same as the original. + ASSERT_TRUE(cloned_dict); + CPDF_Object* cloned_arr = cloned_dict->GetObjectBy("arr"); + ASSERT_TRUE(cloned_arr); + ASSERT_TRUE(cloned_arr->IsArray()); + EXPECT_EQ(1u, cloned_arr->AsArray()->GetCount()); + // Recursively referenced object is not cloned. + EXPECT_EQ(nullptr, cloned_arr->AsArray()->GetObjectAt(0)); + } +} diff --git a/core/fpdfapi/fpdf_parser/cpdf_reference.cpp b/core/fpdfapi/fpdf_parser/cpdf_reference.cpp index afda50c324..4e7b3786d3 100644 --- a/core/fpdfapi/fpdf_parser/cpdf_reference.cpp +++ b/core/fpdfapi/fpdf_parser/cpdf_reference.cpp @@ -7,6 +7,7 @@ #include "core/fpdfapi/fpdf_parser/include/cpdf_reference.h" #include "core/fpdfapi/fpdf_parser/include/cpdf_indirect_object_holder.h" +#include "third_party/base/stl_util.h" CPDF_Reference::CPDF_Reference(CPDF_IndirectObjectHolder* pDoc, int objnum) : m_pObjList(pDoc), m_RefObjNum(objnum) {} @@ -49,10 +50,19 @@ const CPDF_Reference* CPDF_Reference::AsReference() const { return this; } -CPDF_Object* CPDF_Reference::Clone(FX_BOOL bDirect) const { +CPDF_Object* CPDF_Reference::Clone() const { + return CloneObjectNonCyclic(false); +} + +CPDF_Object* CPDF_Reference::CloneNonCyclic( + bool bDirect, + std::set* pVisited) const { + pVisited->insert(this); if (bDirect) { auto* pDirect = GetDirect(); - return pDirect ? pDirect->Clone(TRUE) : nullptr; + return pDirect && !pdfium::ContainsKey(*pVisited, pDirect) + ? pDirect->CloneNonCyclic(true, pVisited) + : nullptr; } return new CPDF_Reference(m_pObjList, m_RefObjNum); } diff --git a/core/fpdfapi/fpdf_parser/cpdf_stream.cpp b/core/fpdfapi/fpdf_parser/cpdf_stream.cpp index 7e65c25533..58b9767dfb 100644 --- a/core/fpdfapi/fpdf_parser/cpdf_stream.cpp +++ b/core/fpdfapi/fpdf_parser/cpdf_stream.cpp @@ -9,6 +9,7 @@ #include "core/fpdfapi/fpdf_parser/include/cpdf_dictionary.h" #include "core/fpdfapi/fpdf_parser/include/cpdf_stream_acc.h" #include "core/fpdfapi/fpdf_parser/include/fpdf_parser_decode.h" +#include "third_party/base/stl_util.h" CPDF_Stream::CPDF_Stream(uint8_t* pData, uint32_t size, CPDF_Dictionary* pDict) : m_pDict(pDict), @@ -17,6 +18,7 @@ CPDF_Stream::CPDF_Stream(uint8_t* pData, uint32_t size, CPDF_Dictionary* pDict) m_pDataBuf(pData) {} CPDF_Stream::~CPDF_Stream() { + m_ObjNum = kInvalidObjNum; if (IsMemoryBased()) FX_Free(m_pDataBuf); @@ -71,13 +73,22 @@ void CPDF_Stream::InitStream(const uint8_t* pData, m_pDict->SetAtInteger("Length", size); } -CPDF_Object* CPDF_Stream::Clone(FX_BOOL bDirect) const { +CPDF_Object* CPDF_Stream::Clone() const { + return CloneObjectNonCyclic(false); +} + +CPDF_Object* CPDF_Stream::CloneNonCyclic( + bool bDirect, + std::set* pVisited) const { + pVisited->insert(this); CPDF_StreamAcc acc; acc.LoadAllData(this, TRUE); uint32_t streamSize = acc.GetSize(); CPDF_Dictionary* pDict = GetDict(); - if (pDict) - pDict = ToDictionary(pDict->Clone(bDirect)); + if (pDict && !pdfium::ContainsKey(*pVisited, pDict)) { + pDict = ToDictionary( + static_cast(pDict)->CloneNonCyclic(bDirect, pVisited)); + } return new CPDF_Stream(acc.DetachData(), streamSize, pDict); } diff --git a/core/fpdfapi/fpdf_parser/cpdf_string.cpp b/core/fpdfapi/fpdf_parser/cpdf_string.cpp index 4b92596242..b18d484347 100644 --- a/core/fpdfapi/fpdf_parser/cpdf_string.cpp +++ b/core/fpdfapi/fpdf_parser/cpdf_string.cpp @@ -23,7 +23,7 @@ CPDF_Object::Type CPDF_String::GetType() const { return STRING; } -CPDF_Object* CPDF_String::Clone(FX_BOOL bDirect) const { +CPDF_Object* CPDF_String::Clone() const { return new CPDF_String(m_String, m_bHex); } diff --git a/core/fpdfapi/fpdf_parser/include/cpdf_array.h b/core/fpdfapi/fpdf_parser/include/cpdf_array.h index 9bb99da053..8c89a060eb 100644 --- a/core/fpdfapi/fpdf_parser/include/cpdf_array.h +++ b/core/fpdfapi/fpdf_parser/include/cpdf_array.h @@ -7,6 +7,7 @@ #ifndef CORE_FPDFAPI_FPDF_PARSER_INCLUDE_CPDF_ARRAY_H_ #define CORE_FPDFAPI_FPDF_PARSER_INCLUDE_CPDF_ARRAY_H_ +#include #include #include "core/fpdfapi/fpdf_parser/include/cpdf_indirect_object_holder.h" @@ -23,7 +24,7 @@ class CPDF_Array : public CPDF_Object { // CPDF_Object. Type GetType() const override; - CPDF_Object* Clone(FX_BOOL bDirect = FALSE) const override; + CPDF_Object* Clone() const override; bool IsArray() const override; CPDF_Array* AsArray() override; const CPDF_Array* AsArray() const override; @@ -68,6 +69,10 @@ class CPDF_Array : public CPDF_Object { protected: ~CPDF_Array() override; + CPDF_Object* CloneNonCyclic( + bool bDirect, + std::set* pVisited) const override; + std::vector m_Objects; }; diff --git a/core/fpdfapi/fpdf_parser/include/cpdf_dictionary.h b/core/fpdfapi/fpdf_parser/include/cpdf_dictionary.h index 29846366d9..be79737add 100644 --- a/core/fpdfapi/fpdf_parser/include/cpdf_dictionary.h +++ b/core/fpdfapi/fpdf_parser/include/cpdf_dictionary.h @@ -8,6 +8,7 @@ #define CORE_FPDFAPI_FPDF_PARSER_INCLUDE_CPDF_DICTIONARY_H_ #include +#include #include "core/fpdfapi/fpdf_parser/include/cpdf_object.h" #include "core/fxcrt/include/fx_coordinates.h" @@ -24,7 +25,7 @@ class CPDF_Dictionary : public CPDF_Object { // CPDF_Object. Type GetType() const override; - CPDF_Object* Clone(FX_BOOL bDirect = FALSE) const override; + CPDF_Object* Clone() const override; CPDF_Dictionary* GetDict() const override; bool IsDictionary() const override; CPDF_Dictionary* AsDictionary() override; @@ -85,6 +86,10 @@ class CPDF_Dictionary : public CPDF_Object { protected: ~CPDF_Dictionary() override; + CPDF_Object* CloneNonCyclic( + bool bDirect, + std::set* visited) const override; + std::map m_Map; }; diff --git a/core/fpdfapi/fpdf_parser/include/cpdf_name.h b/core/fpdfapi/fpdf_parser/include/cpdf_name.h index 523238c892..3cefa7e8c1 100644 --- a/core/fpdfapi/fpdf_parser/include/cpdf_name.h +++ b/core/fpdfapi/fpdf_parser/include/cpdf_name.h @@ -15,7 +15,7 @@ class CPDF_Name : public CPDF_Object { // CPDF_Object. Type GetType() const override; - CPDF_Object* Clone(FX_BOOL bDirect = FALSE) const override; + CPDF_Object* Clone() const override; CFX_ByteString GetString() const override; CFX_WideString GetUnicodeText() const override; void SetString(const CFX_ByteString& str) override; diff --git a/core/fpdfapi/fpdf_parser/include/cpdf_number.h b/core/fpdfapi/fpdf_parser/include/cpdf_number.h index eaa4009543..f3d9042b62 100644 --- a/core/fpdfapi/fpdf_parser/include/cpdf_number.h +++ b/core/fpdfapi/fpdf_parser/include/cpdf_number.h @@ -20,7 +20,7 @@ class CPDF_Number : public CPDF_Object { // CPDF_Object. Type GetType() const override; - CPDF_Object* Clone(FX_BOOL bDirect = FALSE) const override; + CPDF_Object* Clone() const override; CFX_ByteString GetString() const override; FX_FLOAT GetNumber() const override; int GetInteger() const override; diff --git a/core/fpdfapi/fpdf_parser/include/cpdf_object.h b/core/fpdfapi/fpdf_parser/include/cpdf_object.h index f637e36667..8d9bb01119 100644 --- a/core/fpdfapi/fpdf_parser/include/cpdf_object.h +++ b/core/fpdfapi/fpdf_parser/include/cpdf_object.h @@ -7,6 +7,8 @@ #ifndef CORE_FPDFAPI_FPDF_PARSER_INCLUDE_CPDF_OBJECT_H_ #define CORE_FPDFAPI_FPDF_PARSER_INCLUDE_CPDF_OBJECT_H_ +#include + #include "core/fxcrt/include/fx_string.h" #include "core/fxcrt/include/fx_system.h" @@ -39,7 +41,11 @@ class CPDF_Object { uint32_t GetObjNum() const { return m_ObjNum; } uint32_t GetGenNum() const { return m_GenNum; } - virtual CPDF_Object* Clone(FX_BOOL bDirect = FALSE) const = 0; + // Create a deep copy of the object. + virtual CPDF_Object* Clone() const = 0; + // Create a deep copy of the object except any reference object be + // copied to the object it points to directly. + virtual CPDF_Object* CloneDirectObject() const; virtual CPDF_Object* GetDirect() const; void Release(); @@ -79,16 +85,33 @@ class CPDF_Object { virtual const CPDF_String* AsString() const; protected: + friend class CPDF_Array; + friend class CPDF_Dictionary; + friend class CPDF_Document; + friend class CPDF_IndirectObjectHolder; + friend class CPDF_Parser; + friend class CPDF_Reference; + friend class CPDF_Stream; + CPDF_Object() : m_ObjNum(0), m_GenNum(0) {} virtual ~CPDF_Object(); void Destroy() { delete this; } + CPDF_Object* CloneObjectNonCyclic(bool bDirect) const; + + // Create a deep copy of the object with the option to either + // copy a reference object or directly copy the object it refers to + // when |bDirect| is true. + // Also check cyclic reference against |pVisited|, no copy if it is found. + // Complex objects should implement their own CloneNonCyclic() + // function to properly check for possible loop. + virtual CPDF_Object* CloneNonCyclic( + bool bDirect, + std::set* pVisited) const; + uint32_t m_ObjNum; uint32_t m_GenNum; - friend class CPDF_IndirectObjectHolder; - friend class CPDF_Parser; - private: CPDF_Object(const CPDF_Object& src) {} }; diff --git a/core/fpdfapi/fpdf_parser/include/cpdf_reference.h b/core/fpdfapi/fpdf_parser/include/cpdf_reference.h index 49b698eacb..388c1697a8 100644 --- a/core/fpdfapi/fpdf_parser/include/cpdf_reference.h +++ b/core/fpdfapi/fpdf_parser/include/cpdf_reference.h @@ -7,6 +7,8 @@ #ifndef CORE_FPDFAPI_FPDF_PARSER_INCLUDE_CPDF_REFERENCE_H_ #define CORE_FPDFAPI_FPDF_PARSER_INCLUDE_CPDF_REFERENCE_H_ +#include + #include "core/fpdfapi/fpdf_parser/include/cpdf_object.h" class CPDF_IndirectObjectHolder; @@ -17,7 +19,7 @@ class CPDF_Reference : public CPDF_Object { // CPDF_Object. Type GetType() const override; - CPDF_Object* Clone(FX_BOOL bDirect = FALSE) const override; + CPDF_Object* Clone() const override; CPDF_Object* GetDirect() const override; CFX_ByteString GetString() const override; FX_FLOAT GetNumber() const override; @@ -36,6 +38,9 @@ class CPDF_Reference : public CPDF_Object { protected: ~CPDF_Reference() override; + CPDF_Object* CloneNonCyclic( + bool bDirect, + std::set* pVisited) const override; CPDF_Object* SafeGetDirect() const { CPDF_Object* obj = GetDirect(); if (!obj || obj->IsReference()) diff --git a/core/fpdfapi/fpdf_parser/include/cpdf_stream.h b/core/fpdfapi/fpdf_parser/include/cpdf_stream.h index 6aa8bca8dc..7ea761ef51 100644 --- a/core/fpdfapi/fpdf_parser/include/cpdf_stream.h +++ b/core/fpdfapi/fpdf_parser/include/cpdf_stream.h @@ -7,6 +7,8 @@ #ifndef CORE_FPDFAPI_FPDF_PARSER_INCLUDE_CPDF_STREAM_H_ #define CORE_FPDFAPI_FPDF_PARSER_INCLUDE_CPDF_STREAM_H_ +#include + #include "core/fpdfapi/fpdf_parser/include/cpdf_dictionary.h" #include "core/fpdfapi/fpdf_parser/include/cpdf_object.h" #include "core/fxcrt/include/fx_stream.h" @@ -17,7 +19,7 @@ class CPDF_Stream : public CPDF_Object { // CPDF_Object. Type GetType() const override; - CPDF_Object* Clone(FX_BOOL bDirect = FALSE) const override; + CPDF_Object* Clone() const override; CPDF_Dictionary* GetDict() const override; CFX_WideString GetUnicodeText() const override; bool IsStream() const override; @@ -45,6 +47,9 @@ class CPDF_Stream : public CPDF_Object { static const uint32_t kMemoryBasedGenNum = (uint32_t)-1; ~CPDF_Stream() override; + CPDF_Object* CloneNonCyclic( + bool bDirect, + std::set* pVisited) const override; void InitStreamInternal(CPDF_Dictionary* pDict); diff --git a/core/fpdfapi/fpdf_parser/include/cpdf_string.h b/core/fpdfapi/fpdf_parser/include/cpdf_string.h index c17cc182f7..740465c59f 100644 --- a/core/fpdfapi/fpdf_parser/include/cpdf_string.h +++ b/core/fpdfapi/fpdf_parser/include/cpdf_string.h @@ -19,7 +19,7 @@ class CPDF_String : public CPDF_Object { // CPDF_Object. Type GetType() const override; - CPDF_Object* Clone(FX_BOOL bDirect = FALSE) const override; + CPDF_Object* Clone() const override; CFX_ByteString GetString() const override; CFX_WideString GetUnicodeText() const override; void SetString(const CFX_ByteString& str) override; diff --git a/core/fpdfdoc/cpdf_interform.cpp b/core/fpdfdoc/cpdf_interform.cpp index cf728a7d4b..a6f73c620b 100644 --- a/core/fpdfdoc/cpdf_interform.cpp +++ b/core/fpdfdoc/cpdf_interform.cpp @@ -1403,7 +1403,7 @@ CPDF_FormField* CPDF_InterForm::AddTerminalField(CPDF_Dictionary* pFieldDict) { pField = new CPDF_FormField(this, pParent); CPDF_Object* pTObj = pDict->GetObjectBy("T"); if (ToReference(pTObj)) { - CPDF_Object* pClone = pTObj->Clone(TRUE); + CPDF_Object* pClone = pTObj->CloneDirectObject(); if (pClone) pDict->SetAt("T", pClone); else @@ -1535,7 +1535,7 @@ CFDF_Document* CPDF_InterForm::ExportToFDF( } else { CPDF_Object* pV = FPDF_GetFieldAttr(pField->m_pDict, "V"); if (pV) - pFieldDict->SetAt("V", pV->Clone(TRUE)); + pFieldDict->SetAt("V", pV->CloneDirectObject()); } pFields->Add(pFieldDict); } @@ -1587,8 +1587,8 @@ void CPDF_InterForm::FDF_ImportField(CPDF_Dictionary* pFieldDict, CPDF_FormField::Type eType = pField->GetType(); if ((eType == CPDF_FormField::ListBox || eType == CPDF_FormField::ComboBox) && pFieldDict->KeyExist("Opt")) { - pField->m_pDict->SetAt("Opt", - pFieldDict->GetDirectObjectBy("Opt")->Clone(TRUE)); + pField->m_pDict->SetAt( + "Opt", pFieldDict->GetDirectObjectBy("Opt")->CloneDirectObject()); } if (bNotify && m_pFormNotify) { diff --git a/fpdfsdk/fpdfppo.cpp b/fpdfsdk/fpdfppo.cpp index 97fd390cb3..85dba5e975 100644 --- a/fpdfsdk/fpdfppo.cpp +++ b/fpdfsdk/fpdfppo.cpp @@ -407,6 +407,6 @@ DLLEXPORT FPDF_BOOL STDCALL FPDF_CopyViewerPreferences(FPDF_DOCUMENT dest_doc, if (!pDstDict) return FALSE; - pDstDict->SetAt("ViewerPreferences", pSrcDict->Clone(TRUE)); + pDstDict->SetAt("ViewerPreferences", pSrcDict->CloneDirectObject()); return TRUE; } -- cgit v1.2.3