From a11549a566ceed28bc9f6ba100b0d3f6ae59a1f4 Mon Sep 17 00:00:00 2001
From: Jay Berkenbilt <ejb@ql.org>
Date: Sat, 21 Feb 2015 18:22:32 -0500
Subject: Detect loops in /Pages structure

Pushing inherited objects to pages and getting all pages were both
prone to stack overflow infinite loops if there were loops in the
Pages dictionary. There is a general weakness in the code in that any
part of the code that traverses the Pages structure would be prone to
this and would have to implement its own loop detection. A more robust
fix may provide some general method for handling the Pages structure,
but it's probably not worth doing.

Note: addition of *Internal2 private functions was done rather than
changing signatures of existing methods to avoid breaking
compatibility.
---
 libqpdf/QPDF_optimization.cc | 29 +++++++++++++++++++++++++++--
 libqpdf/QPDF_pages.cc        | 21 ++++++++++++++++++++-
 2 files changed, 47 insertions(+), 3 deletions(-)

(limited to 'libqpdf')

diff --git a/libqpdf/QPDF_optimization.cc b/libqpdf/QPDF_optimization.cc
index 5299d90f..fad710d0 100644
--- a/libqpdf/QPDF_optimization.cc
+++ b/libqpdf/QPDF_optimization.cc
@@ -174,6 +174,30 @@ QPDF::pushInheritedAttributesToPageInternal(
     std::vector<QPDFObjectHandle>& pages,
     bool allow_changes, bool warn_skipped_keys)
 {
+    std::set<QPDFObjGen> visited;
+    pushInheritedAttributesToPageInternal2(
+        cur_pages, key_ancestors, pages, allow_changes,
+        warn_skipped_keys, visited);
+}
+
+void
+QPDF::pushInheritedAttributesToPageInternal2(
+    QPDFObjectHandle cur_pages,
+    std::map<std::string, std::vector<QPDFObjectHandle> >& key_ancestors,
+    std::vector<QPDFObjectHandle>& pages,
+    bool allow_changes, bool warn_skipped_keys,
+    std::set<QPDFObjGen>& visited)
+{
+    QPDFObjGen this_og = cur_pages.getObjGen();
+    if (visited.count(this_og) > 0)
+    {
+        throw QPDFExc(
+            qpdf_e_pages, this->file->getName(),
+            this->last_object_description, 0,
+            "Loop detected in /Pages structure (inherited attributes)");
+    }
+    visited.insert(this_og);
+
     // Extract the underlying dictionary object
     std::string type = cur_pages.getKey("/Type").getName();
 
@@ -259,9 +283,9 @@ QPDF::pushInheritedAttributesToPageInternal(
 	int n = kids.getArrayNItems();
 	for (int i = 0; i < n; ++i)
 	{
-            pushInheritedAttributesToPageInternal(
+            pushInheritedAttributesToPageInternal2(
                 kids.getArrayItem(i), key_ancestors, pages,
-                allow_changes, warn_skipped_keys);
+                allow_changes, warn_skipped_keys, visited);
 	}
 
 	// For each inheritable key, pop the stack.  If the stack
@@ -318,6 +342,7 @@ QPDF::pushInheritedAttributesToPageInternal(
 		      this->file->getLastOffset(),
 		      "invalid Type " + type + " in page tree");
     }
+    visited.erase(this_og);
 }
 
 void
diff --git a/libqpdf/QPDF_pages.cc b/libqpdf/QPDF_pages.cc
index 44db064c..f9b421c6 100644
--- a/libqpdf/QPDF_pages.cc
+++ b/libqpdf/QPDF_pages.cc
@@ -56,6 +56,24 @@ void
 QPDF::getAllPagesInternal(QPDFObjectHandle cur_pages,
 			  std::vector<QPDFObjectHandle>& result)
 {
+    std::set<QPDFObjGen> visited;
+    getAllPagesInternal2(cur_pages, result, visited);
+}
+
+void
+QPDF::getAllPagesInternal2(QPDFObjectHandle cur_pages,
+			  std::vector<QPDFObjectHandle>& result,
+                          std::set<QPDFObjGen>& visited)
+{
+    QPDFObjGen this_og = cur_pages.getObjGen();
+    if (visited.count(this_og) > 0)
+    {
+        throw QPDFExc(
+            qpdf_e_pages, this->file->getName(),
+            this->last_object_description, 0,
+            "Loop detected in /Pages structure (getAllPages)");
+    }
+    visited.insert(this_og);
     std::string type;
     QPDFObjectHandle type_key = cur_pages.getKey("/Type");
     if (type_key.isName())
@@ -76,7 +94,7 @@ QPDF::getAllPagesInternal(QPDFObjectHandle cur_pages,
 	int n = kids.getArrayNItems();
 	for (int i = 0; i < n; ++i)
 	{
-	    getAllPagesInternal(kids.getArrayItem(i), result);
+	    getAllPagesInternal2(kids.getArrayItem(i), result, visited);
 	}
     }
     else if (type == "/Page")
@@ -90,6 +108,7 @@ QPDF::getAllPagesInternal(QPDFObjectHandle cur_pages,
 		      this->file->getLastOffset(),
 		      "invalid Type " + type + " in page tree");
     }
+    visited.erase(this_og);
 }
 
 void
-- 
cgit v1.2.3-54-g00ecf