[Libreoffice-commits] core.git: Branch 'libreoffice-6-1' - include/vcl vcl/source

[Libreoffice Gerrit user]

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

New commits:
commit f232f2591b6be35ca3722e8e52fbdf36865a5967
Author:     Caolán McNamara <caolanm at redhat.com>
AuthorDate: Wed Aug 1 15:05:45 2018 +0100
Commit:     Michael Stahl <Michael.Stahl at cib.de>
CommitDate: Fri Aug 3 11:07:53 2018 +0200

    forcepoint#66 protect against infinite parse recurse
    
    Change-Id: I0313cc141469a00b7d6a5bd15400e9d5a8f686cf
    Reviewed-on: https://gerrit.libreoffice.org/58449
    Tested-by: Jenkins
    Reviewed-by: Michael Stahl <Michael.Stahl at cib.de>

diff --git a/include/vcl/filter/pdfdocument.hxx b/include/vcl/filter/pdfdocument.hxx
index b19bfca6b408..03180fd0597f 100644
--- a/include/vcl/filter/pdfdocument.hxx
+++ b/include/vcl/filter/pdfdocument.hxx
@@ -51,9 +51,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() = default;
+    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.
@@ -63,7 +75,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;
@@ -123,9 +134,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 7d4efc9e0eb0..3d9008a22943 100644
--- a/vcl/source/filter/ipdf/pdfdocument.cxx
+++ b/vcl/source/filter/ipdf/pdfdocument.cxx
@@ -1815,16 +1815,16 @@ size_t PDFDocument::GetObjectOffset(size_t nIndex) const
 const std::vector<std::unique_ptr<PDFElement>>& PDFDocument::GetElements() { return m_aElements; }
 
 /// 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())
     {
@@ -1837,7 +1837,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;
@@ -1846,13 +1846,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()
@@ -1897,7 +1897,7 @@ std::vector<PDFObjectElement*> PDFDocument::GetPages()
         return aRet;
     }
 
-    pPages->visitPages(aRet);
+    visitPages(pPages, aRet);
 
     return aRet;
 }
@@ -2133,7 +2133,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)
@@ -2163,6 +2162,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);
@@ -2208,7 +2209,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);
@@ -2386,6 +2387,8 @@ size_t PDFDictionaryElement::Parse(const std::vector<std::unique_ptr<PDFElement>
         aNumbers.clear();
     }
 
+    pThis->setParsing(false);
+
     return nRet;
 }