Fix colorspace loading for mutually referencing colorspaces.

For example, Indexed colorspace A uses ICC Based colorspace B as its
"base". B declares A as its "Alternate" fallback.

Bug: chromium:759012
Change-Id: I4b78e68b9a77456050ecae4452837495546bf93d
Reviewed-on: https://pdfium-review.googlesource.com/12471
Commit-Queue: Henrique Nakashima <hnakashima@chromium.org>
Reviewed-by: dsinclair <dsinclair@chromium.org>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>

6 files changed, 94 insertions, 40 deletions

diff --git a/core/fpdfapi/page/cpdf_colorspace.cpp b/core/fpdfapi/page/cpdf_colorspace.cpp
index 0769687f34..aee7316bdf 100644
--- a/core/fpdfapi/page/cpdf_colorspace.cpp
+++ b/core/fpdfapi/page/cpdf_colorspace.cpp
@@ -75,7 +75,9 @@ class CPDF_CalGray : public CPDF_ColorSpace {
   ~CPDF_CalGray() override {}
 
   // CPDF_ColorSpace:
-  bool v_Load(CPDF_Document* pDoc, CPDF_Array* pArray) override;
+  bool v_Load(CPDF_Document* pDoc,
+              CPDF_Array* pArray,
+              std::set<CPDF_Object*>* pVisited) override;
   bool GetRGB(float* pBuf, float* R, float* G, float* B) const override;
   void TranslateImageLine(uint8_t* pDestBuf,
                           const uint8_t* pSrcBuf,
@@ -95,7 +97,9 @@ class CPDF_CalRGB : public CPDF_ColorSpace {
   explicit CPDF_CalRGB(CPDF_Document* pDoc);
   ~CPDF_CalRGB() override {}
 
-  bool v_Load(CPDF_Document* pDoc, CPDF_Array* pArray) override;
+  bool v_Load(CPDF_Document* pDoc,
+              CPDF_Array* pArray,
+              std::set<CPDF_Object*>* pVisited) override;
 
   bool GetRGB(float* pBuf, float* R, float* G, float* B) const override;
 
@@ -119,7 +123,9 @@ class CPDF_LabCS : public CPDF_ColorSpace {
   explicit CPDF_LabCS(CPDF_Document* pDoc);
   ~CPDF_LabCS() override {}
 
-  bool v_Load(CPDF_Document* pDoc, CPDF_Array* pArray) override;
+  bool v_Load(CPDF_Document* pDoc,
+              CPDF_Array* pArray,
+              std::set<CPDF_Object*>* pVisited) override;
 
   void GetDefaultValue(int iComponent,
                        float* value,
@@ -145,7 +151,9 @@ class CPDF_ICCBasedCS : public CPDF_ColorSpace {
   ~CPDF_ICCBasedCS() override;
 
   // CPDF_ColorSpace:
-  bool v_Load(CPDF_Document* pDoc, CPDF_Array* pArray) override;
+  bool v_Load(CPDF_Document* pDoc,
+              CPDF_Array* pArray,
+              std::set<CPDF_Object*>* pVisited) override;
   bool GetRGB(float* pBuf, float* R, float* G, float* B) const override;
   void EnableStdConversion(bool bEnabled) override;
   void TranslateImageLine(uint8_t* pDestBuf,
@@ -159,7 +167,9 @@ class CPDF_ICCBasedCS : public CPDF_ColorSpace {
 
  private:
   // If no valid ICC profile or using sRGB, try looking for an alternate.
-  bool FindAlternateProfile(CPDF_Document* pDoc, CPDF_Dictionary* pDict);
+  bool FindAlternateProfile(CPDF_Document* pDoc,
+                            CPDF_Dictionary* pDict,
+                            std::set<CPDF_Object*>* pVisited);
 
   void UseStockAlternateProfile();
   bool IsValidComponents(int32_t nComps) const;
@@ -176,7 +186,9 @@ class CPDF_IndexedCS : public CPDF_ColorSpace {
   explicit CPDF_IndexedCS(CPDF_Document* pDoc);
   ~CPDF_IndexedCS() override;
 
-  bool v_Load(CPDF_Document* pDoc, CPDF_Array* pArray) override;
+  bool v_Load(CPDF_Document* pDoc,
+              CPDF_Array* pArray,
+              std::set<CPDF_Object*>* pVisited) override;
 
   bool GetRGB(float* pBuf, float* R, float* G, float* B) const override;
 
@@ -200,7 +212,9 @@ class CPDF_SeparationCS : public CPDF_ColorSpace {
                        float* value,
                        float* min,
                        float* max) const override;
-  bool v_Load(CPDF_Document* pDoc, CPDF_Array* pArray) override;
+  bool v_Load(CPDF_Document* pDoc,
+              CPDF_Array* pArray,
+              std::set<CPDF_Object*>* pVisited) override;
   bool GetRGB(float* pBuf, float* R, float* G, float* B) const override;
   void EnableStdConversion(bool bEnabled) override;
 
@@ -219,7 +233,9 @@ class CPDF_DeviceNCS : public CPDF_ColorSpace {
                        float* value,
                        float* min,
                        float* max) const override;
-  bool v_Load(CPDF_Document* pDoc, CPDF_Array* pArray) override;
+  bool v_Load(CPDF_Document* pDoc,
+              CPDF_Array* pArray,
+              std::set<CPDF_Object*>* pVisited) override;
   bool GetRGB(float* pBuf, float* R, float* G, float* B) const override;
   void EnableStdConversion(bool bEnabled) override;
 
@@ -367,6 +383,14 @@ CPDF_ColorSpace* CPDF_ColorSpace::GetStockCS(int family) {
 
 std::unique_ptr<CPDF_ColorSpace> CPDF_ColorSpace::Load(CPDF_Document* pDoc,
                                                        CPDF_Object* pObj) {
+  std::set<CPDF_Object*> visited;
+  return Load(pDoc, pObj, &visited);
+}
+
+std::unique_ptr<CPDF_ColorSpace> CPDF_ColorSpace::Load(
+    CPDF_Document* pDoc,
+    CPDF_Object* pObj,
+    std::set<CPDF_Object*>* pVisited) {
   if (!pObj)
     return nullptr;
 
@@ -433,7 +457,7 @@ std::unique_ptr<CPDF_ColorSpace> CPDF_ColorSpace::Load(CPDF_Document* pDoc,
       return nullptr;
   }
   pCS->m_pArray = pArray;
-  if (!pCS->v_Load(pDoc, pArray))
+  if (!pCS->v_Load(pDoc, pArray, pVisited))
     return nullptr;
 
   return pCS;
@@ -525,14 +549,18 @@ CPDF_ColorSpace::CPDF_ColorSpace(CPDF_Document* pDoc,
 
 CPDF_ColorSpace::~CPDF_ColorSpace() {}
 
-bool CPDF_ColorSpace::v_Load(CPDF_Document* pDoc, CPDF_Array* pArray) {
+bool CPDF_ColorSpace::v_Load(CPDF_Document* pDoc,
+                             CPDF_Array* pArray,
+                             std::set<CPDF_Object*>* pVisited) {
   return true;
 }
 
 CPDF_CalGray::CPDF_CalGray(CPDF_Document* pDoc)
     : CPDF_ColorSpace(pDoc, PDFCS_CALGRAY, 1) {}
 
-bool CPDF_CalGray::v_Load(CPDF_Document* pDoc, CPDF_Array* pArray) {
+bool CPDF_CalGray::v_Load(CPDF_Document* pDoc,
+                          CPDF_Array* pArray,
+                          std::set<CPDF_Object*>* pVisited) {
   CPDF_Dictionary* pDict = pArray->GetDictAt(1);
   if (!pDict)
     return false;
@@ -575,7 +603,9 @@ void CPDF_CalGray::TranslateImageLine(uint8_t* pDestBuf,
 CPDF_CalRGB::CPDF_CalRGB(CPDF_Document* pDoc)
     : CPDF_ColorSpace(pDoc, PDFCS_CALRGB, 3) {}
 
-bool CPDF_CalRGB::v_Load(CPDF_Document* pDoc, CPDF_Array* pArray) {
+bool CPDF_CalRGB::v_Load(CPDF_Document* pDoc,
+                         CPDF_Array* pArray,
+                         std::set<CPDF_Object*>* pVisited) {
   CPDF_Dictionary* pDict = pArray->GetDictAt(1);
   if (!pDict)
     return false;
@@ -682,7 +712,9 @@ void CPDF_LabCS::GetDefaultValue(int iComponent,
   *value = pdfium::clamp(0.0f, *min, *max);
 }
 
-bool CPDF_LabCS::v_Load(CPDF_Document* pDoc, CPDF_Array* pArray) {
+bool CPDF_LabCS::v_Load(CPDF_Document* pDoc,
+                        CPDF_Array* pArray,
+                        std::set<CPDF_Object*>* pVisited) {
   CPDF_Dictionary* pDict = pArray->GetDictAt(1);
   if (!pDict)
     return false;
@@ -774,7 +806,9 @@ CPDF_ICCBasedCS::~CPDF_ICCBasedCS() {
   }
 }
 
-bool CPDF_ICCBasedCS::v_Load(CPDF_Document* pDoc, CPDF_Array* pArray) {
+bool CPDF_ICCBasedCS::v_Load(CPDF_Document* pDoc,
+                             CPDF_Array* pArray,
+                             std::set<CPDF_Object*>* pVisited) {
   CPDF_Stream* pStream = pArray->GetStreamAt(1);
   if (!pStream)
     return false;
@@ -802,7 +836,8 @@ bool CPDF_ICCBasedCS::v_Load(CPDF_Document* pDoc, CPDF_Array* pArray) {
   // If PDFium does not understand the ICC profile format at all, or if it's
   // SRGB, a profile PDFium recognizes but does not support well, then try the
   // alternate profile.
-  if (!m_pProfile->IsSupported() && !FindAlternateProfile(pDoc, pDict)) {
+  if (!m_pProfile->IsSupported() &&
+      !FindAlternateProfile(pDoc, pDict, pVisited)) {
     // If there is no alternate profile, use a stock profile as mentioned in
     // the PDF 1.7 spec in table 4.16 in the "Alternate" key description.
     UseStockAlternateProfile();
@@ -899,12 +934,13 @@ void CPDF_ICCBasedCS::TranslateImageLine(uint8_t* pDestBuf,
 }
 
 bool CPDF_ICCBasedCS::FindAlternateProfile(CPDF_Document* pDoc,
-                                           CPDF_Dictionary* pDict) {
+                                           CPDF_Dictionary* pDict,
+                                           std::set<CPDF_Object*>* pVisited) {
   CPDF_Object* pAlterCSObj = pDict->GetDirectObjectFor("Alternate");
   if (!pAlterCSObj)
     return false;
 
-  auto pAlterCS = CPDF_ColorSpace::Load(pDoc, pAlterCSObj);
+  auto pAlterCS = CPDF_ColorSpace::Load(pDoc, pAlterCSObj, pVisited);
   if (!pAlterCS)
     return false;
 
@@ -958,7 +994,9 @@ CPDF_IndexedCS::~CPDF_IndexedCS() {
   }
 }
 
-bool CPDF_IndexedCS::v_Load(CPDF_Document* pDoc, CPDF_Array* pArray) {
+bool CPDF_IndexedCS::v_Load(CPDF_Document* pDoc,
+                            CPDF_Array* pArray,
+                            std::set<CPDF_Object*>* pVisited) {
   if (pArray->GetCount() < 4)
     return false;
 
@@ -967,7 +1005,7 @@ bool CPDF_IndexedCS::v_Load(CPDF_Document* pDoc, CPDF_Array* pArray) {
     return false;
 
   CPDF_DocPageData* pDocPageData = pDoc->GetPageData();
-  m_pBaseCS = pDocPageData->GetColorSpace(pBaseObj, nullptr);
+  m_pBaseCS = pDocPageData->GetColorSpaceGuarded(pBaseObj, nullptr, pVisited);
   if (!m_pBaseCS)
     return false;
 
@@ -1039,7 +1077,9 @@ void CPDF_SeparationCS::GetDefaultValue(int iComponent,
   *max = 1.0f;
 }
 
-bool CPDF_SeparationCS::v_Load(CPDF_Document* pDoc, CPDF_Array* pArray) {
+bool CPDF_SeparationCS::v_Load(CPDF_Document* pDoc,
+                               CPDF_Array* pArray,
+                               std::set<CPDF_Object*>* pVisited) {
   CFX_ByteString name = pArray->GetStringAt(1);
   if (name == "None") {
     m_Type = None;
@@ -1117,7 +1157,9 @@ void CPDF_DeviceNCS::GetDefaultValue(int iComponent,
   *max = 1.0f;
 }
 
-bool CPDF_DeviceNCS::v_Load(CPDF_Document* pDoc, CPDF_Array* pArray) {
+bool CPDF_DeviceNCS::v_Load(CPDF_Document* pDoc,
+                            CPDF_Array* pArray,
+                            std::set<CPDF_Object*>* pVisited) {
   CPDF_Array* pObj = ToArray(pArray->GetDirectObjectAt(1));
   if (!pObj)
     return false;
diff --git a/core/fpdfapi/page/cpdf_colorspace.h b/core/fpdfapi/page/cpdf_colorspace.h
index a518689ded..31dd32629b 100644
--- a/core/fpdfapi/page/cpdf_colorspace.h
+++ b/core/fpdfapi/page/cpdf_colorspace.h
@@ -8,6 +8,7 @@
 #define CORE_FPDFAPI_PAGE_CPDF_COLORSPACE_H_
 
 #include <memory>
+#include <set>
 
 #include "core/fpdfapi/page/cpdf_pattern.h"
 #include "core/fxcrt/cfx_unowned_ptr.h"
@@ -44,6 +45,10 @@ class CPDF_ColorSpace {
   static CPDF_ColorSpace* ColorspaceFromName(const CFX_ByteString& name);
   static std::unique_ptr<CPDF_ColorSpace> Load(CPDF_Document* pDoc,
                                                CPDF_Object* pCSObj);
+  static std::unique_ptr<CPDF_ColorSpace> Load(
+      CPDF_Document* pDoc,
+      CPDF_Object* pCSObj,
+      std::set<CPDF_Object*>* pVisited);
 
   void Release();
 
@@ -74,7 +79,9 @@ class CPDF_ColorSpace {
   CPDF_ColorSpace(CPDF_Document* pDoc, int family, uint32_t nComponents);
   virtual ~CPDF_ColorSpace();
 
-  virtual bool v_Load(CPDF_Document* pDoc, CPDF_Array* pArray);
+  virtual bool v_Load(CPDF_Document* pDoc,
+                      CPDF_Array* pArray,
+                      std::set<CPDF_Object*>* pVisited);
 
   CFX_UnownedPtr<CPDF_Document> const m_pDocument;
   int m_Family;
diff --git a/core/fpdfapi/page/cpdf_docpagedata.cpp b/core/fpdfapi/page/cpdf_docpagedata.cpp
index 71143d0b6c..1af9d6ee22 100644
--- a/core/fpdfapi/page/cpdf_docpagedata.cpp
+++ b/core/fpdfapi/page/cpdf_docpagedata.cpp
@@ -212,10 +212,10 @@ CPDF_ColorSpace* CPDF_DocPageData::GetColorSpace(
     CPDF_Object* pCSObj,
     const CPDF_Dictionary* pResources) {
   std::set<CPDF_Object*> visited;
-  return GetColorSpaceImpl(pCSObj, pResources, &visited);
+  return GetColorSpaceGuarded(pCSObj, pResources, &visited);
 }
 
-CPDF_ColorSpace* CPDF_DocPageData::GetColorSpaceImpl(
+CPDF_ColorSpace* CPDF_DocPageData::GetColorSpaceGuarded(
     CPDF_Object* pCSObj,
     const CPDF_Dictionary* pResources,
     std::set<CPDF_Object*>* pVisited) {
@@ -225,15 +225,16 @@ CPDF_ColorSpace* CPDF_DocPageData::GetColorSpaceImpl(
   if (pdfium::ContainsKey(*pVisited, pCSObj))
     return nullptr;
 
+  pdfium::ScopedSetInsertion<CPDF_Object*> insertion(pVisited, pCSObj);
+
   if (pCSObj->IsName()) {
     CFX_ByteString name = pCSObj->GetString();
     CPDF_ColorSpace* pCS = CPDF_ColorSpace::ColorspaceFromName(name);
     if (!pCS && pResources) {
       CPDF_Dictionary* pList = pResources->GetDictFor("ColorSpace");
       if (pList) {
-        pdfium::ScopedSetInsertion<CPDF_Object*> insertion(pVisited, pCSObj);
-        return GetColorSpaceImpl(pList->GetDirectObjectFor(name), nullptr,
-                                 pVisited);
+        return GetColorSpaceGuarded(pList->GetDirectObjectFor(name), nullptr,
+                                    pVisited);
       }
     }
     if (!pCS || !pResources)
@@ -258,8 +259,7 @@ CPDF_ColorSpace* CPDF_DocPageData::GetColorSpaceImpl(
     if (!pDefaultCS)
       return pCS;
 
-    pdfium::ScopedSetInsertion<CPDF_Object*> insertion(pVisited, pCSObj);
-    return GetColorSpaceImpl(pDefaultCS, nullptr, pVisited);
+    return GetColorSpaceGuarded(pDefaultCS, nullptr, pVisited);
   }
 
   CPDF_Array* pArray = pCSObj->AsArray();
@@ -267,9 +267,8 @@ CPDF_ColorSpace* CPDF_DocPageData::GetColorSpaceImpl(
     return nullptr;
 
   if (pArray->GetCount() == 1) {
-    pdfium::ScopedSetInsertion<CPDF_Object*> insertion(pVisited, pCSObj);
-    return GetColorSpaceImpl(pArray->GetDirectObjectAt(0), pResources,
-                             pVisited);
+    return GetColorSpaceGuarded(pArray->GetDirectObjectAt(0), pResources,
+                                pVisited);
   }
 
   CPDF_CountedColorSpace* csData = nullptr;
@@ -282,7 +281,7 @@ CPDF_ColorSpace* CPDF_DocPageData::GetColorSpaceImpl(
   }
 
   std::unique_ptr<CPDF_ColorSpace> pCS =
-      CPDF_ColorSpace::Load(m_pPDFDoc.Get(), pArray);
+      CPDF_ColorSpace::Load(m_pPDFDoc.Get(), pArray, pVisited);
   if (!pCS)
     return nullptr;
 
diff --git a/core/fpdfapi/page/cpdf_docpagedata.h b/core/fpdfapi/page/cpdf_docpagedata.h
index 0fda16911d..40647bc9a6 100644
--- a/core/fpdfapi/page/cpdf_docpagedata.h
+++ b/core/fpdfapi/page/cpdf_docpagedata.h
@@ -41,6 +41,10 @@ class CPDF_DocPageData {
 
   CPDF_ColorSpace* GetColorSpace(CPDF_Object* pCSObj,
                                  const CPDF_Dictionary* pResources);
+  CPDF_ColorSpace* GetColorSpaceGuarded(CPDF_Object* pCSObj,
+                                        const CPDF_Dictionary* pResources,
+                                        std::set<CPDF_Object*>* pVisited);
+
   CPDF_ColorSpace* GetCopiedColorSpace(CPDF_Object* pCSObj);
   void ReleaseColorSpace(const CPDF_Object* pColorSpace);
 
@@ -64,10 +68,6 @@ class CPDF_DocPageData {
  private:
   using CPDF_CountedFont = CPDF_CountedObject<CPDF_Font>;
 
-  CPDF_ColorSpace* GetColorSpaceImpl(CPDF_Object* pCSObj,
-                                     const CPDF_Dictionary* pResources,
-                                     std::set<CPDF_Object*>* pVisited);
-
   bool m_bForceClear;
   CFX_UnownedPtr<CPDF_Document> const m_pPDFDoc;
   std::map<CFX_ByteString, CPDF_Stream*> m_HashProfileMap;
diff --git a/core/fpdfapi/page/cpdf_patterncs.cpp b/core/fpdfapi/page/cpdf_patterncs.cpp
index 46b975ab67..d1b7bf8e10 100644
--- a/core/fpdfapi/page/cpdf_patterncs.cpp
+++ b/core/fpdfapi/page/cpdf_patterncs.cpp
@@ -24,13 +24,15 @@ CPDF_PatternCS::~CPDF_PatternCS() {
   }
 }
 
-bool CPDF_PatternCS::v_Load(CPDF_Document* pDoc, CPDF_Array* pArray) {
+bool CPDF_PatternCS::v_Load(CPDF_Document* pDoc,
+                            CPDF_Array* pArray,
+                            std::set<CPDF_Object*>* pVisited) {
   CPDF_Object* pBaseCS = pArray->GetDirectObjectAt(1);
   if (pBaseCS == m_pArray)
     return false;
 
   CPDF_DocPageData* pDocPageData = pDoc->GetPageData();
-  m_pBaseCS = pDocPageData->GetColorSpace(pBaseCS, nullptr);
+  m_pBaseCS = pDocPageData->GetColorSpaceGuarded(pBaseCS, nullptr, pVisited);
   if (!m_pBaseCS) {
     m_nComponents = 1;
     return true;
diff --git a/core/fpdfapi/page/cpdf_patterncs.h b/core/fpdfapi/page/cpdf_patterncs.h
index ff5ad827cc..288c6f6d96 100644
--- a/core/fpdfapi/page/cpdf_patterncs.h
+++ b/core/fpdfapi/page/cpdf_patterncs.h
@@ -7,6 +7,8 @@
 #ifndef CORE_FPDFAPI_PAGE_CPDF_PATTERNCS_H_
 #define CORE_FPDFAPI_PAGE_CPDF_PATTERNCS_H_
 
+#include <set>
+
 #include "core/fpdfapi/page/cpdf_colorspace.h"
 
 class CPDF_Document;
@@ -17,7 +19,9 @@ class CPDF_PatternCS : public CPDF_ColorSpace {
   ~CPDF_PatternCS() override;
 
   // CPDF_ColorSpace:
-  bool v_Load(CPDF_Document* pDoc, CPDF_Array* pArray) override;
+  bool v_Load(CPDF_Document* pDoc,
+              CPDF_Array* pArray,
+              std::set<CPDF_Object*>* pVisited) override;
   bool GetRGB(float* pBuf, float* R, float* G, float* B) const override;
 
  private: