Security: better bounds checks for linearization data

The faulty code was only used during explicit checks of linearization
data.  Those checks are not part of normal reading or writing of PDF
files.

7 files changed, 44 insertions, 1 deletions

diff --git a/ChangeLog b/ChangeLog
index 7440f632..8a10865f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,12 @@
 2013-10-05  Jay Berkenbilt  <ejb@ql.org>
 
+	* Security fix: avoid buffer overrun that could be caused by bogus
+	data in linearization hint streams.  The incorrect code could only
+	be triggered when checking linearization data, which must be
+	invoked explicitly.  qpdf does not check linearization data when
+	reading or writing linearized files, but the qpdf --check command
+	does check linearization data.
+
 	* Security fix: properly handle empty strings in
 	QPDF_Name::normalizeName.  The empty string is not a valid name
 	and would never be parsed as a name, so there were no known
diff --git a/libqpdf/QPDF_linearization.cc b/libqpdf/QPDF_linearization.cc
index 2c4fefc0..dd09b1c0 100644
--- a/libqpdf/QPDF_linearization.cc
+++ b/libqpdf/QPDF_linearization.cc
@@ -295,11 +295,25 @@ QPDF::readLinearizationData()
     readHPageOffset(BitStream(h_buf, h_size));
 
     int HSi = HS.getIntValue();
+    if ((HSi < 0) || (HSi >= h_size))
+    {
+        throw QPDFExc(qpdf_e_damaged_pdf, this->file->getName(),
+                      "linearization hint table",
+                      this->file->getLastOffset(),
+                      "/S (shared object) offset is out of bounds");
+    }
     readHSharedObject(BitStream(h_buf + HSi, h_size - HSi));
 
     if (HO.isInteger())
     {
 	int HOi = HO.getIntValue();
+        if ((HOi < 0) || (HOi >= h_size))
+        {
+            throw QPDFExc(qpdf_e_damaged_pdf, this->file->getName(),
+                          "linearization hint table",
+                          this->file->getLastOffset(),
+                          "/O (outline) offset is out of bounds");
+        }
 	readHGeneric(BitStream(h_buf + HOi, h_size - HOi),
 		     this->outline_hints);
     }
diff --git a/qpdf/qtest/qpdf.test b/qpdf/qtest/qpdf.test
index a30dd7b4..87c9d8c1 100644
--- a/qpdf/qtest/qpdf.test
+++ b/qpdf/qtest/qpdf.test
@@ -199,7 +199,7 @@ $td->runtest("remove page we don't have",
 show_ntests();
 # ----------
 $td->notify("--- Miscellaneous Tests ---");
-$n_tests += 67;
+$n_tests += 69;
 
 $td->runtest("qpdf version",
 	     {$td->COMMAND => "qpdf --version"},
@@ -527,6 +527,16 @@ $td->runtest("ignore broken decode parms with no filters",
              {$td->FILE => "broken-decode-parms-no-filter.out",
               $td->EXIT_STATUS => 0},
              $td->NORMALIZE_NEWLINES);
+$td->runtest("bounds check linearization data 1",
+             {$td->COMMAND => "qpdf --check linearization-bounds-1.pdf"},
+             {$td->FILE => "linearization-bounds-1.out",
+              $td->EXIT_STATUS => 2},
+             $td->NORMALIZE_NEWLINES);
+$td->runtest("bounds check linearization data 2",
+             {$td->COMMAND => "qpdf --check linearization-bounds-2.pdf"},
+             {$td->FILE => "linearization-bounds-2.out",
+              $td->EXIT_STATUS => 2},
+             $td->NORMALIZE_NEWLINES);
 
 show_ntests();
 # ----------
diff --git a/qpdf/qtest/qpdf/linearization-bounds-1.out b/qpdf/qtest/qpdf/linearization-bounds-1.out
new file mode 100644
index 00000000..eaeef14c
--- /dev/null
+++ b/qpdf/qtest/qpdf/linearization-bounds-1.out
@@ -0,0 +1,6 @@
+checking linearization-bounds-1.pdf
+PDF Version: 1.3
+File is not encrypted
+File is linearized
+WARNING: linearization-bounds-1.pdf (linearization hint stream: object 62 0, file position 1183): attempting to recover stream length
+linearization-bounds-1.pdf (linearization hint table, file position 1183): /S (shared object) offset is out of bounds
diff --git a/qpdf/qtest/qpdf/linearization-bounds-1.pdf b/qpdf/qtest/qpdf/linearization-bounds-1.pdf
new file mode 100644
index 00000000..44befc22
--- /dev/null
+++ b/qpdf/qtest/qpdf/linearization-bounds-1.pdf
diff --git a/qpdf/qtest/qpdf/linearization-bounds-2.out b/qpdf/qtest/qpdf/linearization-bounds-2.out
new file mode 100644
index 00000000..bdf7c91b
--- /dev/null
+++ b/qpdf/qtest/qpdf/linearization-bounds-2.out
@@ -0,0 +1,6 @@
+checking linearization-bounds-2.pdf
+PDF Version: 1.3
+File is not encrypted
+File is linearized
+WARNING: linearization-bounds-2.pdf (linearization hint stream: object 62 0, file position 1183): attempting to recover stream length
+linearization-bounds-2.pdf (linearization hint table, file position 1183): /S (shared object) offset is out of bounds
diff --git a/qpdf/qtest/qpdf/linearization-bounds-2.pdf b/qpdf/qtest/qpdf/linearization-bounds-2.pdf
new file mode 100644
index 00000000..bdd6177c
--- /dev/null
+++ b/qpdf/qtest/qpdf/linearization-bounds-2.pdf