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.

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.
Jay Berkenbilt
1 parent b097d7a8
Showing 7 changed files with 44 additions and 1 deletions
ChangeLog
libqpdf/QPDF_linearization.cc
qpdf/qtest/qpdf.test
qpdf/qtest/qpdf/linearization-bounds-1.out
qpdf/qtest/qpdf/linearization-bounds-1.pdf
qpdf/qtest/qpdf/linearization-bounds-2.out
qpdf/qtest/qpdf/linearization-bounds-2.pdf
 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
@@ -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);
     }
@@ -199,7 +199,7 @@ $td-&gt;runtest(&quot;remove page we don&#39;t have&quot;,
 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-&gt;runtest(&quot;ignore broken decode parms with no filters&quot;,
              {$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();
 # ----------
+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
+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