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_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 +- 8 files changed, 54 insertions(+), 11 deletions(-) (limited to 'core/fpdfapi/fpdf_parser/include') 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; -- cgit v1.2.3