From 6bc997a89429662668fbdce2a0d217a93e8a1be1 Mon Sep 17 00:00:00 2001 From: Wei Li Date: Tue, 19 Jan 2016 12:35:03 -0800 Subject: Merge to XFA: Fix infinite loops caused by calling circular indirect objects There are multiple functions in CPDF_Object class which can cause infinite loop due to recursively calling circular indirect objects. Fix them by deference indirect object first. BUG=pdfium:355 TBR=jun_fang@foxitsoftware.com, thestig@chromium.org Review URL: https://codereview.chromium.org/1585533002 . (cherry picked from commit 90853cb1dfd1bf3803ec21cfae3e93948137be61) Review URL: https://codereview.chromium.org/1602103004 . --- core/include/fpdfapi/fpdf_objects.h | 35 ++---- .../fpdfapi/fpdf_parser/fpdf_parser_objects.cpp | 135 ++++++++------------- fpdfsdk/src/fpdfview_embeddertest.cpp | 5 + testing/resources/bug_355.pdf | 15 +++ 4 files changed, 79 insertions(+), 111 deletions(-) create mode 100644 testing/resources/bug_355.pdf diff --git a/core/include/fpdfapi/fpdf_objects.h b/core/include/fpdfapi/fpdf_objects.h index 92ce0b922f..fdf11720f4 100644 --- a/core/include/fpdfapi/fpdf_objects.h +++ b/core/include/fpdfapi/fpdf_objects.h @@ -45,44 +45,31 @@ struct PARSE_CONTEXT; class CPDF_Object { public: int GetType() const { return m_Type; } - FX_DWORD GetObjNum() const { return m_ObjNum; } - FX_DWORD GetGenNum() const { return m_GenNum; } FX_BOOL IsIdentical(CPDF_Object* pObj) const; - CPDF_Object* Clone(FX_BOOL bDirect = FALSE) const; - CPDF_Object* CloneRef(CPDF_IndirectObjectHolder* pObjs) const; CPDF_Object* GetDirect() const; + int GetDirectType() const; + FX_BOOL IsModified() const { return FALSE; } void Release(); CFX_ByteString GetString() const; - CFX_ByteStringC GetConstString() const; - CFX_WideString GetUnicodeText(CFX_CharMap* pCharMap = NULL) const; FX_FLOAT GetNumber() const; - FX_FLOAT GetNumber16() const; - int GetInteger() const; - CPDF_Dictionary* GetDict() const; - CPDF_Array* GetArray() const; void SetString(const CFX_ByteString& str); - void SetUnicodeText(const FX_WCHAR* pUnicodes, int len = -1); - int GetDirectType() const; - - FX_BOOL IsModified() const { return FALSE; } - bool IsArray() const { return m_Type == PDFOBJ_ARRAY; } bool IsBoolean() const { return m_Type == PDFOBJ_BOOLEAN; } bool IsDictionary() const { return m_Type == PDFOBJ_DICTIONARY; } @@ -94,25 +81,18 @@ class CPDF_Object { CPDF_Array* AsArray(); const CPDF_Array* AsArray() const; - CPDF_Boolean* AsBoolean(); const CPDF_Boolean* AsBoolean() const; - CPDF_Dictionary* AsDictionary(); const CPDF_Dictionary* AsDictionary() const; - CPDF_Name* AsName(); const CPDF_Name* AsName() const; - CPDF_Number* AsNumber(); const CPDF_Number* AsNumber() const; - CPDF_Reference* AsReference(); const CPDF_Reference* AsReference() const; - CPDF_Stream* AsStream(); const CPDF_Stream* AsStream() const; - CPDF_String* AsString(); const CPDF_String* AsString() const; @@ -122,8 +102,8 @@ class CPDF_Object { ~CPDF_Object() {} void Destroy(); - static const int kObjectRefMaxDepth = 128; - static int s_nCurRefDepth; + const CPDF_Object* const GetBasicObject() const; + FX_DWORD m_Type; FX_DWORD m_ObjNum; FX_DWORD m_GenNum; @@ -147,6 +127,9 @@ class CPDF_Boolean : public CPDF_Object { return m_bValue == pOther->m_bValue; } + CFX_ByteString GetString() const { return m_bValue ? "true" : "false"; } + FX_BOOL GetValue() const { return m_bValue; } + protected: FX_BOOL m_bValue; friend class CPDF_Object; @@ -216,7 +199,7 @@ class CPDF_String : public CPDF_Object { explicit CPDF_String(const CFX_WideString& str); - CFX_ByteString& GetString() { return m_String; } + CFX_ByteString GetString() const { return m_String; } FX_BOOL Identical(CPDF_String* pOther) const { return m_String == pOther->m_String; @@ -246,7 +229,7 @@ class CPDF_Name : public CPDF_Object { explicit CPDF_Name(const FX_CHAR* str) : CPDF_Object(PDFOBJ_NAME), m_Name(str) {} - CFX_ByteString& GetString() { return m_Name; } + CFX_ByteString GetString() const { return m_Name; } FX_BOOL Identical(CPDF_Name* pOther) const { return m_Name == pOther->m_Name; diff --git a/core/src/fpdfapi/fpdf_parser/fpdf_parser_objects.cpp b/core/src/fpdfapi/fpdf_parser/fpdf_parser_objects.cpp index cad8d7701d..a3b0568931 100644 --- a/core/src/fpdfapi/fpdf_parser/fpdf_parser_objects.cpp +++ b/core/src/fpdfapi/fpdf_parser/fpdf_parser_objects.cpp @@ -18,9 +18,6 @@ const FX_DWORD kBlockSize = 1024; } // namespace -// static -int CPDF_Object::s_nCurRefDepth = 0; - void CPDF_Object::Release() { if (m_ObjNum) { return; @@ -48,65 +45,55 @@ void CPDF_Object::Destroy() { delete this; } } -CFX_ByteString CPDF_Object::GetString() const { - switch (m_Type) { - case PDFOBJ_BOOLEAN: - return AsBoolean()->m_bValue ? "true" : "false"; - case PDFOBJ_NUMBER: - return AsNumber()->GetString(); - case PDFOBJ_STRING: - return AsString()->m_String; - case PDFOBJ_NAME: - return AsName()->m_Name; - case PDFOBJ_REFERENCE: { - const CPDF_Reference* pRef = AsReference(); - if (!pRef->m_pObjList) - break; - CPDF_Object* pObj = - pRef->m_pObjList->GetIndirectObject(pRef->GetRefObjNum(), nullptr); - return pObj ? pObj->GetString() : CFX_ByteString(); +const CPDF_Object* const CPDF_Object::GetBasicObject() const { + const CPDF_Reference* pRef = AsReference(); + if (!pRef) { + // This is not an indirect reference. + return this; + } + if (!pRef->m_pObjList) + return nullptr; + return pRef->m_pObjList->GetIndirectObject(pRef->GetRefObjNum(), nullptr); +} + +CFX_ByteString CPDF_Object::GetString() const { + const CPDF_Object* obj = GetBasicObject(); + if (obj) { + switch (obj->GetType()) { + case PDFOBJ_BOOLEAN: + return obj->AsBoolean()->GetString(); + case PDFOBJ_NUMBER: + return obj->AsNumber()->GetString(); + case PDFOBJ_STRING: + return obj->AsString()->GetString(); + case PDFOBJ_NAME: + return obj->AsName()->GetString(); } } return CFX_ByteString(); } + CFX_ByteStringC CPDF_Object::GetConstString() const { - switch (m_Type) { - case PDFOBJ_STRING: { - CFX_ByteString str = AsString()->m_String; - return CFX_ByteStringC((const uint8_t*)str, str.GetLength()); - } - case PDFOBJ_NAME: { - CFX_ByteString name = AsName()->m_Name; - return CFX_ByteStringC((const uint8_t*)name, name.GetLength()); + const CPDF_Object* obj = GetBasicObject(); + if (obj) { + FX_DWORD type = obj->GetType(); + if (type == PDFOBJ_STRING) { + CFX_ByteString str = obj->AsString()->GetString(); + return CFX_ByteStringC(str); } - case PDFOBJ_REFERENCE: { - const CPDF_Reference* pRef = AsReference(); - if (!pRef->m_pObjList) - break; - - CPDF_Object* pObj = - pRef->m_pObjList->GetIndirectObject(pRef->GetRefObjNum(), nullptr); - return pObj ? pObj->GetConstString() : CFX_ByteStringC(); + if (type == PDFOBJ_NAME) { + CFX_ByteString name = obj->AsName()->GetString(); + return CFX_ByteStringC(name); } } return CFX_ByteStringC(); } FX_FLOAT CPDF_Object::GetNumber() const { - switch (m_Type) { - case PDFOBJ_NUMBER: - return AsNumber()->GetNumber(); - case PDFOBJ_REFERENCE: { - const CPDF_Reference* pRef = AsReference(); - if (!pRef->m_pObjList) - break; - - CPDF_Object* pObj = - pRef->m_pObjList->GetIndirectObject(pRef->GetRefObjNum(), nullptr); - return pObj ? pObj->GetNumber() : 0; - } - } + const CPDF_Object* obj = GetBasicObject(); + if (obj && obj->GetType() == PDFOBJ_NUMBER) + return AsNumber()->GetNumber(); return 0; } @@ -115,52 +102,30 @@ FX_FLOAT CPDF_Object::GetNumber16() const { } int CPDF_Object::GetInteger() const { - CFX_AutoRestorer restorer(&s_nCurRefDepth); - if (++s_nCurRefDepth > kObjectRefMaxDepth) - return 0; - - switch (m_Type) { - case PDFOBJ_BOOLEAN: - return AsBoolean()->m_bValue; - case PDFOBJ_NUMBER: - return AsNumber()->GetInteger(); - case PDFOBJ_REFERENCE: { - const CPDF_Reference* pRef = AsReference(); - PARSE_CONTEXT context; - FXSYS_memset(&context, 0, sizeof(PARSE_CONTEXT)); - if (!pRef->m_pObjList) - return 0; - - CPDF_Object* pObj = - pRef->m_pObjList->GetIndirectObject(pRef->GetRefObjNum(), &context); - return pObj ? pObj->GetInteger() : 0; - } + const CPDF_Object* obj = GetBasicObject(); + if (obj) { + FX_DWORD type = obj->GetType(); + if (type == PDFOBJ_BOOLEAN) + return obj->AsBoolean()->GetValue(); + if (type == PDFOBJ_NUMBER) + return obj->AsNumber()->GetInteger(); } return 0; } CPDF_Dictionary* CPDF_Object::GetDict() const { - switch (m_Type) { - case PDFOBJ_DICTIONARY: + const CPDF_Object* obj = GetBasicObject(); + if (obj) { + FX_DWORD type = obj->GetType(); + if (type == PDFOBJ_DICTIONARY) { // The method should be made non-const if we want to not be const. // See bug #234. return const_cast(AsDictionary()); - case PDFOBJ_STREAM: - return AsStream()->GetDict(); - case PDFOBJ_REFERENCE: { - const CPDF_Reference* pRef = AsReference(); - CPDF_IndirectObjectHolder* pIndirect = pRef->GetObjList(); - if (!pIndirect) - return nullptr; - CPDF_Object* pObj = - pIndirect->GetIndirectObject(pRef->GetRefObjNum(), nullptr); - if (!pObj || (pObj == this)) - return nullptr; - return pObj->GetDict(); } - default: - return nullptr; + if (type == PDFOBJ_STREAM) + return AsStream()->GetDict(); } + return nullptr; } CPDF_Array* CPDF_Object::GetArray() const { diff --git a/fpdfsdk/src/fpdfview_embeddertest.cpp b/fpdfsdk/src/fpdfview_embeddertest.cpp index 09d1f07e82..da7313dd5f 100644 --- a/fpdfsdk/src/fpdfview_embeddertest.cpp +++ b/fpdfsdk/src/fpdfview_embeddertest.cpp @@ -235,6 +235,11 @@ TEST_F(FPDFViewEmbeddertest, Hang_344) { EXPECT_FALSE(OpenDocument("bug_344.pdf")); } +// The test should pass when there is no infinite recursion in +// CPDF_SyntaxParser::GetString(). +TEST_F(FPDFViewEmbeddertest, Hang_355) { + EXPECT_FALSE(OpenDocument("bug_355.pdf")); +} // The test should pass even when the file has circular references to pages. TEST_F(FPDFViewEmbeddertest, Hang_360) { EXPECT_FALSE(OpenDocument("bug_360.pdf")); diff --git a/testing/resources/bug_355.pdf b/testing/resources/bug_355.pdf new file mode 100644 index 0000000000..0fc53b050e --- /dev/null +++ b/testing/resources/bug_355.pdf @@ -0,0 +1,15 @@ +%P%PDF-1%PDF-1.4 +%Ǐ +6 0 obj%<> +%PDF-%PDF-2.6 % +1 /(0 er '/obFl \ No newline at end of file -- cgit v1.2.3