[Backport] Fix for CVE-2019-5762

Make CPDF_ContentMarkItem stop caching the properties dict.

It could be aliased with some other dictionary in the file. We
note that the dictionary one level up will always be an indirect
object in the sharing case, and indirect objects are persisted
by the IndirectObjectHolder, so hold a pointer to that and retrieve
the specific property_name field on the fly.

Bug: chromium:900552
Change-Id: I2e300020d6a7191648dd139a485b6d284e259976
Reviewed-on: https://pdfium-review.googlesource.com/c/44970
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Michael Brüning <michael.bruning@qt.io>

6 files changed, 47 insertions, 42 deletions

diff --git a/chromium/third_party/pdfium/core/fpdfapi/page/cpdf_contentmark.cpp b/chromium/third_party/pdfium/core/fpdfapi/page/cpdf_contentmark.cpp
index 725a17348b7..a5909ce9e5c 100644
--- a/chromium/third_party/pdfium/core/fpdfapi/page/cpdf_contentmark.cpp
+++ b/chromium/third_party/pdfium/core/fpdfapi/page/cpdf_contentmark.cpp
@@ -55,12 +55,12 @@ void CPDF_ContentMark::AddMarkWithDirectDict(ByteString name,
   m_pMarkData->AddMarkWithDirectDict(std::move(name), pDict);
 }
 
-void CPDF_ContentMark::AddMarkWithPropertiesDict(
-    ByteString name,
+void CPDF_ContentMark::AddMarkWithPropertiesHolder(
+    const ByteString& name,
     CPDF_Dictionary* pDict,
     const ByteString& property_name) {
   EnsureMarkDataExists();
-  m_pMarkData->AddMarkWithPropertiesDict(std::move(name), pDict, property_name);
+  m_pMarkData->AddMarkWithPropertiesHolder(name, pDict, property_name);
 }
 
 bool CPDF_ContentMark::RemoveMark(CPDF_ContentMarkItem* pMarkItem) {
@@ -145,12 +145,12 @@ void CPDF_ContentMark::MarkData::AddMarkWithDirectDict(ByteString name,
   m_Marks.push_back(pItem);
 }
 
-void CPDF_ContentMark::MarkData::AddMarkWithPropertiesDict(
-    ByteString name,
+void CPDF_ContentMark::MarkData::AddMarkWithPropertiesHolder(
+    const ByteString& name,
     CPDF_Dictionary* pDict,
     const ByteString& property_name) {
   auto pItem = pdfium::MakeRetain<CPDF_ContentMarkItem>(std::move(name));
-  pItem->SetPropertiesDict(pDict, property_name);
+  pItem->SetPropertiesHolder(pDict, property_name);
   m_Marks.push_back(pItem);
 }
 
diff --git a/chromium/third_party/pdfium/core/fpdfapi/page/cpdf_contentmark.h b/chromium/third_party/pdfium/core/fpdfapi/page/cpdf_contentmark.h
index dc4cc08838b..c3dd60b5baa 100644
--- a/chromium/third_party/pdfium/core/fpdfapi/page/cpdf_contentmark.h
+++ b/chromium/third_party/pdfium/core/fpdfapi/page/cpdf_contentmark.h
@@ -32,9 +32,9 @@ class CPDF_ContentMark {
 
   void AddMark(ByteString name);
   void AddMarkWithDirectDict(ByteString name, CPDF_Dictionary* pDict);
-  void AddMarkWithPropertiesDict(ByteString name,
-                                 CPDF_Dictionary* pDict,
-                                 const ByteString& property_name);
+  void AddMarkWithPropertiesHolder(const ByteString& name,
+                                   CPDF_Dictionary* pHolder,
+                                   const ByteString& property_name);
   bool RemoveMark(CPDF_ContentMarkItem* pMarkItem);
   void DeleteLastMark();
   size_t FindFirstDifference(const CPDF_ContentMark* other) const;
@@ -54,9 +54,9 @@ class CPDF_ContentMark {
     int GetMarkedContentID() const;
     void AddMark(ByteString name);
     void AddMarkWithDirectDict(ByteString name, CPDF_Dictionary* pDict);
-    void AddMarkWithPropertiesDict(ByteString name,
-                                   CPDF_Dictionary* pDict,
-                                   const ByteString& property_name);
+    void AddMarkWithPropertiesHolder(const ByteString& name,
+                                     CPDF_Dictionary* pHolder,
+                                     const ByteString& property_name);
     bool RemoveMark(CPDF_ContentMarkItem* pMarkItem);
     void DeleteLastMark();
 
diff --git a/chromium/third_party/pdfium/core/fpdfapi/page/cpdf_contentmarkitem.cpp b/chromium/third_party/pdfium/core/fpdfapi/page/cpdf_contentmarkitem.cpp
index 8eba4aa15f0..3347a1d149f 100644
--- a/chromium/third_party/pdfium/core/fpdfapi/page/cpdf_contentmarkitem.cpp
+++ b/chromium/third_party/pdfium/core/fpdfapi/page/cpdf_contentmarkitem.cpp
@@ -18,7 +18,7 @@ CPDF_ContentMarkItem::~CPDF_ContentMarkItem() {}
 const CPDF_Dictionary* CPDF_ContentMarkItem::GetParam() const {
   switch (m_ParamType) {
     case PropertiesDict:
-      return m_pPropertiesDict.Get();
+      return m_pPropertiesHolder->GetDictFor(m_PropertyName);
     case DirectDict:
       return m_pDirectDict.get();
     case None:
@@ -43,9 +43,10 @@ void CPDF_ContentMarkItem::SetDirectDict(
   m_pDirectDict = std::move(pDict);
 }
 
-void CPDF_ContentMarkItem::SetPropertiesDict(CPDF_Dictionary* pDict,
-                                             const ByteString& property_name) {
+void CPDF_ContentMarkItem::SetPropertiesHolder(
+    CPDF_Dictionary* pHolder,
+    const ByteString& property_name) {
   m_ParamType = PropertiesDict;
-  m_pPropertiesDict = pDict;
+  m_pPropertiesHolder = pHolder;
   m_PropertyName = property_name;
 }
diff --git a/chromium/third_party/pdfium/core/fpdfapi/page/cpdf_contentmarkitem.h b/chromium/third_party/pdfium/core/fpdfapi/page/cpdf_contentmarkitem.h
index 435aef4b10d..1fd41901103 100644
--- a/chromium/third_party/pdfium/core/fpdfapi/page/cpdf_contentmarkitem.h
+++ b/chromium/third_party/pdfium/core/fpdfapi/page/cpdf_contentmarkitem.h
@@ -32,13 +32,13 @@ class CPDF_ContentMarkItem : public Retainable {
   bool HasMCID() const;
 
   void SetDirectDict(std::unique_ptr<CPDF_Dictionary> pDict);
-  void SetPropertiesDict(CPDF_Dictionary* pDict,
-                         const ByteString& property_name);
+  void SetPropertiesHolder(CPDF_Dictionary* pHolder,
+                           const ByteString& property_name);
 
  private:
   ByteString m_MarkName;
   ParamType m_ParamType = None;
-  UnownedPtr<CPDF_Dictionary> m_pPropertiesDict;
+  UnownedPtr<CPDF_Dictionary> m_pPropertiesHolder;
   ByteString m_PropertyName;
   std::unique_ptr<CPDF_Dictionary> m_pDirectDict;
 };
diff --git a/chromium/third_party/pdfium/core/fpdfapi/page/cpdf_streamcontentparser.cpp b/chromium/third_party/pdfium/core/fpdfapi/page/cpdf_streamcontentparser.cpp
index 860f6d6b3ba..7cb4be450ab 100644
--- a/chromium/third_party/pdfium/core/fpdfapi/page/cpdf_streamcontentparser.cpp
+++ b/chromium/third_party/pdfium/core/fpdfapi/page/cpdf_streamcontentparser.cpp
@@ -602,30 +602,26 @@ void CPDF_StreamContentParser::Handle_EOFillStrokePath() {
 }
 
 void CPDF_StreamContentParser::Handle_BeginMarkedContent_Dictionary() {
-  ByteString tag = GetString(1);
   CPDF_Object* pProperty = GetObject(0);
   if (!pProperty)
     return;
 
-  bool bIndirect = pProperty->IsName();
-  ByteString property_name;
-  if (bIndirect) {
-    property_name = pProperty->GetString();
-    pProperty = FindResourceObj("Properties", property_name);
-    if (!pProperty)
+  ByteString tag = GetString(1);
+  std::unique_ptr<CPDF_ContentMark> new_marks =
+      m_ContentMarksStack.top()->Clone();
+
+  if (pProperty->IsName()) {
+    ByteString property_name = pProperty->GetString();
+    CPDF_Dictionary* pHolder = FindResourceHolder("Properties");
+    if (!pHolder || !pHolder->GetDictFor(property_name))
       return;
+    new_marks->AddMarkWithPropertiesHolder(tag, pHolder, property_name);
+  } else if (pProperty->IsDictionary()) {
+    new_marks->AddMarkWithDirectDict(tag, pProperty->AsDictionary());
+  } else {
+    return;
   }
-  if (CPDF_Dictionary* pDict = pProperty->AsDictionary()) {
-    std::unique_ptr<CPDF_ContentMark> new_marks =
-        m_ContentMarksStack.top()->Clone();
-    if (bIndirect) {
-      new_marks->AddMarkWithPropertiesDict(std::move(tag), pDict,
-                                           property_name);
-    } else {
-      new_marks->AddMarkWithDirectDict(std::move(tag), pDict);
-    }
-    m_ContentMarksStack.push(std::move(new_marks));
-  }
+  m_ContentMarksStack.push(std::move(new_marks));
 }
 
 void CPDF_StreamContentParser::Handle_BeginImage() {
@@ -1153,18 +1149,25 @@ void CPDF_StreamContentParser::Handle_SetFont() {
   }
 }
 
-CPDF_Object* CPDF_StreamContentParser::FindResourceObj(const ByteString& type,
-                                                       const ByteString& name) {
+CPDF_Dictionary* CPDF_StreamContentParser::FindResourceHolder(
+    const ByteString& type) {
   if (!m_pResources)
     return nullptr;
+
   CPDF_Dictionary* pDict = m_pResources->GetDictFor(type);
   if (pDict)
-    return pDict->GetDirectObjectFor(name);
+    return pDict;
+
   if (m_pResources == m_pPageResources || !m_pPageResources)
     return nullptr;
 
-  CPDF_Dictionary* pPageDict = m_pPageResources->GetDictFor(type);
-  return pPageDict ? pPageDict->GetDirectObjectFor(name) : nullptr;
+  return m_pPageResources->GetDictFor(type);
+}
+
+CPDF_Object* CPDF_StreamContentParser::FindResourceObj(const ByteString& type,
+                                                       const ByteString& name) {
+  CPDF_Dictionary* pHolder = FindResourceHolder(type);
+  return pHolder ? pHolder->GetDirectObjectFor(name) : nullptr;
 }
 
 CPDF_Font* CPDF_StreamContentParser::FindFont(const ByteString& name) {
diff --git a/chromium/third_party/pdfium/core/fpdfapi/page/cpdf_streamcontentparser.h b/chromium/third_party/pdfium/core/fpdfapi/page/cpdf_streamcontentparser.h
index aac66e883be..52b2a44bcfa 100644
--- a/chromium/third_party/pdfium/core/fpdfapi/page/cpdf_streamcontentparser.h
+++ b/chromium/third_party/pdfium/core/fpdfapi/page/cpdf_streamcontentparser.h
@@ -126,6 +126,7 @@ class CPDF_StreamContentParser {
                         bool bGraph);
   CPDF_ColorSpace* FindColorSpace(const ByteString& name);
   CPDF_Pattern* FindPattern(const ByteString& name, bool bShading);
+  CPDF_Dictionary* FindResourceHolder(const ByteString& type);
   CPDF_Object* FindResourceObj(const ByteString& type, const ByteString& name);
 
   // Takes ownership of |pImageObj|, returns unowned pointer to it.