From 9291178596c4e5165de51c0be5a525cbaa564140 Mon Sep 17 00:00:00 2001
From: Caolán McNamara <caolanm@redhat.com>
Date: Wed, 1 Aug 2018 15:05:45 +0100
Subject: forcepoint#66 protect against infinite parse recurse

Reviewed-on: https://gerrit.libreoffice.org/58452
Tested-by: Jenkins
Reviewed-by: Michael Stahl <Michael.Stahl@cib.de>
(cherry picked from commit 171657a1f675268839526b1a13e5f3549fb73516)

Change-Id: I0313cc141469a00b7d6a5bd15400e9d5a8f686cf
---
 include/vcl/filter/pdfdocument.hxx     | 16 ++++++++++++----
 vcl/source/filter/ipdf/pdfdocument.cxx | 21 ++++++++++++---------
 2 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/include/vcl/filter/pdfdocument.hxx b/include/vcl/filter/pdfdocument.hxx
index 48240b7ecb98..b684072fb1b4 100644
--- a/include/vcl/filter/pdfdocument.hxx
+++ b/include/vcl/filter/pdfdocument.hxx
@@ -37,9 +37,21 @@ class PDFNumberElement;
 /// A byte range in a PDF file.
 class VCL_DLLPUBLIC PDFElement
 {
+    bool m_bVisiting;
+    bool m_bParsing;
+
 public:
+    PDFElement()
+        : m_bVisiting(false)
+        , m_bParsing(false)
+    {
+    }
     virtual bool Read(SvStream& rStream) = 0;
     virtual ~PDFElement() { }
+    void setVisiting(bool bVisiting) { m_bVisiting = bVisiting; }
+    bool alreadyVisiting() const { return m_bVisiting; }
+    void setParsing(bool bParsing) { m_bParsing = bParsing; }
+    bool alreadyParsing() const { return m_bParsing; }
 };
 
 /// Indirect object: something with a unique ID.
@@ -49,7 +61,6 @@ class VCL_DLLPUBLIC PDFObjectElement : public PDFElement
     PDFDocument& m_rDoc;
     double m_fObjectValue;
     double m_fGenerationValue;
-    bool m_bVisiting;
     std::map<OString, PDFElement*> m_aDictionary;
     /// If set, the object contains this number element (outside any dictionary/array).
     PDFNumberElement* m_pNumberElement;
@@ -109,9 +120,6 @@ public:
     SvMemoryStream* GetStreamBuffer() const;
     void SetStreamBuffer(std::unique_ptr<SvMemoryStream>& pStreamBuffer);
     PDFDocument& GetDocument();
-
-    /// Visits the page tree recursively, looking for page objects.
-    void visitPages(std::vector<PDFObjectElement*>& rRet);
 };
 
 /// Array object: a list.
diff --git a/vcl/source/filter/ipdf/pdfdocument.cxx b/vcl/source/filter/ipdf/pdfdocument.cxx
index 20a2b951ab6c..83560394ba4c 100644
--- a/vcl/source/filter/ipdf/pdfdocument.cxx
+++ b/vcl/source/filter/ipdf/pdfdocument.cxx
@@ -1769,16 +1769,16 @@ const std::vector< std::unique_ptr<PDFElement> >& PDFDocument::GetElements()
 }
 
 /// Visits the page tree recursively, looking for page objects.
-void PDFObjectElement::visitPages(std::vector<PDFObjectElement*>& rRet)
+static void visitPages(PDFObjectElement* pPages, std::vector<PDFObjectElement*>& rRet)
 {
-    auto pKids = dynamic_cast<PDFArrayElement*>(Lookup("Kids"));
+    auto pKids = dynamic_cast<PDFArrayElement*>(pPages->Lookup("Kids"));
     if (!pKids)
     {
         SAL_WARN("vcl.filter", "visitPages: pages has no kids");
         return;
     }
 
-    m_bVisiting = true;
+    pPages->setVisiting(true);
 
     for (const auto& pKid : pKids->GetElements())
     {
@@ -1791,7 +1791,7 @@ void PDFObjectElement::visitPages(std::vector<PDFObjectElement*>& rRet)
             continue;
 
         // detect if visiting reenters itself
-        if (pKidObject->m_bVisiting)
+        if (pKidObject->alreadyVisiting())
         {
             SAL_WARN("vcl.filter", "visitPages: loop in hierarchy");
             continue;
@@ -1800,13 +1800,13 @@ void PDFObjectElement::visitPages(std::vector<PDFObjectElement*>& rRet)
         auto pName = dynamic_cast<PDFNameElement*>(pKidObject->Lookup("Type"));
         if (pName && pName->GetValue() == "Pages")
             // Pages inside pages: recurse.
-            pKidObject->visitPages(rRet);
+            visitPages(pKidObject, rRet);
         else
             // Found an actual page.
             rRet.push_back(pKidObject);
     }
 
-    m_bVisiting = false;
+    pPages->setVisiting(false);
 }
 
 std::vector<PDFObjectElement*> PDFDocument::GetPages()
@@ -1851,7 +1851,7 @@ std::vector<PDFObjectElement*> PDFDocument::GetPages()
         return aRet;
     }
 
-    pPages->visitPages(aRet);
+    visitPages(pPages, aRet);
 
     return aRet;
 }
@@ -2149,7 +2149,6 @@ PDFObjectElement::PDFObjectElement(PDFDocument& rDoc, double fObjectValue, doubl
     : m_rDoc(rDoc),
       m_fObjectValue(fObjectValue),
       m_fGenerationValue(fGenerationValue),
-      m_bVisiting(false),
       m_pNumberElement(nullptr),
       m_nDictionaryOffset(0),
       m_nDictionaryLength(0),
@@ -2177,6 +2176,8 @@ size_t PDFDictionaryElement::Parse(const std::vector< std::unique_ptr<PDFElement
     if (!rDictionary.empty())
         return nRet;
 
+    pThis->setParsing(true);
+
     auto pThisObject = dynamic_cast<PDFObjectElement*>(pThis);
     // This is set to non-nullptr here for nested dictionaries only.
     auto pThisDictionary = dynamic_cast<PDFDictionaryElement*>(pThis);
@@ -2222,7 +2223,7 @@ size_t PDFDictionaryElement::Parse(const std::vector< std::unique_ptr<PDFElement
                     pThisObject->SetDictionaryOffset(nDictionaryOffset);
                 }
             }
-            else
+            else if (!pDictionary->alreadyParsing())
             {
                 // Nested dictionary.
                 i = PDFDictionaryElement::Parse(rElements, pDictionary, pDictionary->m_aItems);
@@ -2396,6 +2397,8 @@ size_t PDFDictionaryElement::Parse(const std::vector< std::unique_ptr<PDFElement
         aNumbers.clear();
     }
 
+    pThis->setParsing(false);
+
     return nRet;
 }
 
-- 
cgit