summaryrefslogtreecommitdiff
path: root/core
diff options
context:
space:
mode:
authorHenrique Nakashima <hnakashima@chromium.org>2018-04-11 21:55:49 +0000
committerChromium commit bot <commit-bot@chromium.org>2018-04-11 21:55:49 +0000
commitb92ec18fdccd196035e02f3232c0b730637ac815 (patch)
treea78ea05f4b90265dd6451a6a626bdb3a771b689b /core
parentc763970de6e749123af76170c16bbc3929058437 (diff)
downloadpdfium-b92ec18fdccd196035e02f3232c0b730637ac815.tar.xz
Fix crash when ColorSpace references itself directly.
Also fixes any problems with cycles between colorspaces. Past fixes have solved problems with CPDF_DocPageData::GetColorSpace() calling itself and CPDF_DocPageData::GetColorSpace() calling CPDF_ColorSpace::Load() and vice versa. They have not solved CPDF_ColorSpace::Load() calling itself. This CL repurposes the |pVisited| set to ensure CPDF_ColorSpace::Load() does not try to load a colorspace as a dependency of itself and creates |pVisitedLocal| to ensure CPDF_DocPageData::GetColorSpace() does not create a similar circular dependency not involving CPDF_ColorSpace::Load(). Bug: chromium:828206 Change-Id: Ib2d0ec494be169135607f3651e0f70627b26ebd7 Reviewed-on: https://pdfium-review.googlesource.com/29810 Reviewed-by: Lei Zhang <thestig@chromium.org> Commit-Queue: Henrique Nakashima <hnakashima@chromium.org>
Diffstat (limited to 'core')
-rw-r--r--core/fpdfapi/page/cpdf_colorspace.cpp5
-rw-r--r--core/fpdfapi/page/cpdf_docpagedata.cpp24
-rw-r--r--core/fpdfapi/page/cpdf_docpagedata.h16
-rw-r--r--core/fpdfapi/page/cpdf_docpagedata_embeddertest.cpp16
4 files changed, 54 insertions, 7 deletions
diff --git a/core/fpdfapi/page/cpdf_colorspace.cpp b/core/fpdfapi/page/cpdf_colorspace.cpp
index e1dd598a63..528c6ea3cf 100644
--- a/core/fpdfapi/page/cpdf_colorspace.cpp
+++ b/core/fpdfapi/page/cpdf_colorspace.cpp
@@ -400,6 +400,11 @@ std::unique_ptr<CPDF_ColorSpace> CPDF_ColorSpace::Load(
if (!pObj)
return nullptr;
+ if (pdfium::ContainsKey(*pVisited, pObj))
+ return nullptr;
+
+ pdfium::ScopedSetInsertion<CPDF_Object*> insertion(pVisited, pObj);
+
if (pObj->IsName()) {
return std::unique_ptr<CPDF_ColorSpace>(
ColorspaceFromName(pObj->GetString()));
diff --git a/core/fpdfapi/page/cpdf_docpagedata.cpp b/core/fpdfapi/page/cpdf_docpagedata.cpp
index 0d2a9189b6..03db315912 100644
--- a/core/fpdfapi/page/cpdf_docpagedata.cpp
+++ b/core/fpdfapi/page/cpdf_docpagedata.cpp
@@ -219,13 +219,22 @@ CPDF_ColorSpace* CPDF_DocPageData::GetColorSpaceGuarded(
CPDF_Object* pCSObj,
const CPDF_Dictionary* pResources,
std::set<CPDF_Object*>* pVisited) {
+ std::set<CPDF_Object*> visitedLocal;
+ return GetColorSpaceInternal(pCSObj, pResources, pVisited, &visitedLocal);
+}
+
+CPDF_ColorSpace* CPDF_DocPageData::GetColorSpaceInternal(
+ CPDF_Object* pCSObj,
+ const CPDF_Dictionary* pResources,
+ std::set<CPDF_Object*>* pVisited,
+ std::set<CPDF_Object*>* pVisitedInternal) {
if (!pCSObj)
return nullptr;
- if (pdfium::ContainsKey(*pVisited, pCSObj))
+ if (pdfium::ContainsKey(*pVisitedInternal, pCSObj))
return nullptr;
- pdfium::ScopedSetInsertion<CPDF_Object*> insertion(pVisited, pCSObj);
+ pdfium::ScopedSetInsertion<CPDF_Object*> insertion(pVisitedInternal, pCSObj);
if (pCSObj->IsName()) {
ByteString name = pCSObj->GetString();
@@ -233,8 +242,8 @@ CPDF_ColorSpace* CPDF_DocPageData::GetColorSpaceGuarded(
if (!pCS && pResources) {
CPDF_Dictionary* pList = pResources->GetDictFor("ColorSpace");
if (pList) {
- return GetColorSpaceGuarded(pList->GetDirectObjectFor(name), nullptr,
- pVisited);
+ return GetColorSpaceInternal(pList->GetDirectObjectFor(name), nullptr,
+ pVisited, pVisitedInternal);
}
}
if (!pCS || !pResources)
@@ -259,7 +268,8 @@ CPDF_ColorSpace* CPDF_DocPageData::GetColorSpaceGuarded(
if (!pDefaultCS)
return pCS;
- return GetColorSpaceGuarded(pDefaultCS, nullptr, pVisited);
+ return GetColorSpaceInternal(pDefaultCS, nullptr, pVisited,
+ pVisitedInternal);
}
CPDF_Array* pArray = pCSObj->AsArray();
@@ -267,8 +277,8 @@ CPDF_ColorSpace* CPDF_DocPageData::GetColorSpaceGuarded(
return nullptr;
if (pArray->GetCount() == 1) {
- return GetColorSpaceGuarded(pArray->GetDirectObjectAt(0), pResources,
- pVisited);
+ return GetColorSpaceInternal(pArray->GetDirectObjectAt(0), pResources,
+ pVisited, pVisitedInternal);
}
CPDF_CountedColorSpace* csData = nullptr;
diff --git a/core/fpdfapi/page/cpdf_docpagedata.h b/core/fpdfapi/page/cpdf_docpagedata.h
index 02107aa2bb..5508f568be 100644
--- a/core/fpdfapi/page/cpdf_docpagedata.h
+++ b/core/fpdfapi/page/cpdf_docpagedata.h
@@ -39,8 +39,13 @@ class CPDF_DocPageData {
CPDF_FontEncoding* pEncoding);
void ReleaseFont(const CPDF_Dictionary* pFontDict);
+ // Loads a colorspace.
CPDF_ColorSpace* GetColorSpace(CPDF_Object* pCSObj,
const CPDF_Dictionary* pResources);
+
+ // Loads a colorspace in a context that might be while loading another
+ // colorspace. |pVisited| is passed recursively to avoid circular calls
+ // involving CPDF_ColorSpace::Load().
CPDF_ColorSpace* GetColorSpaceGuarded(CPDF_Object* pCSObj,
const CPDF_Dictionary* pResources,
std::set<CPDF_Object*>* pVisited);
@@ -68,6 +73,17 @@ class CPDF_DocPageData {
private:
using CPDF_CountedFont = CPDF_CountedObject<CPDF_Font>;
+ // Loads a colorspace in a context that might be while loading another
+ // colorspace, or even in a recursive call from this method itself. |pVisited|
+ // is passed recursively to avoid circular calls involving
+ // CPDF_ColorSpace::Load() and |pVisitedInternal| is also passed recursively
+ // to avoid circular calls with this method calling itself.
+ CPDF_ColorSpace* GetColorSpaceInternal(
+ CPDF_Object* pCSObj,
+ const CPDF_Dictionary* pResources,
+ std::set<CPDF_Object*>* pVisited,
+ std::set<CPDF_Object*>* pVisitedInternal);
+
bool m_bForceClear;
UnownedPtr<CPDF_Document> const m_pPDFDoc;
std::map<ByteString, CPDF_Stream*> m_HashProfileMap;
diff --git a/core/fpdfapi/page/cpdf_docpagedata_embeddertest.cpp b/core/fpdfapi/page/cpdf_docpagedata_embeddertest.cpp
new file mode 100644
index 0000000000..5120b21c56
--- /dev/null
+++ b/core/fpdfapi/page/cpdf_docpagedata_embeddertest.cpp
@@ -0,0 +1,16 @@
+// Copyright 2018 PDFium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "testing/embedder_test.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+class CPDF_DocPageDataEmbeddertest : public EmbedderTest {};
+
+TEST_F(CPDF_DocPageDataEmbeddertest, Crasher_828206) {
+ EXPECT_TRUE(OpenDocument("bug_828206.pdf"));
+ FPDF_PAGE page = LoadPage(0);
+ ASSERT_TRUE(page);
+ RenderLoadedPage(page);
+ UnloadPage(page);
+}