diff options
| -rw-r--r-- | ChangeLog | 4 | ||||
| -rw-r--r-- | TODO | 3 | ||||
| -rw-r--r-- | include/qpdf/QPDF.hh | 9 | ||||
| -rw-r--r-- | libqpdf/QPDF_optimization.cc | 29 | ||||
| -rw-r--r-- | libqpdf/QPDF_pages.cc | 21 | ||||
| -rw-r--r-- | qpdf/qtest/qpdf.test | 6 | ||||
| -rw-r--r-- | qpdf/qtest/qpdf/pages-loop.out | 5 | ||||
| -rw-r--r-- | qpdf/qtest/qpdf/pages-loop.pdf | 102 |
8 files changed, 175 insertions, 4 deletions
@@ -1,5 +1,9 @@ 2015-02-21 Jay Berkenbilt <ejb@ql.org> + * Detect loops in Pages structure. Thanks to Gynvael Coldwind and + Mateusz Jurczyk of the Google Security Team for providing a sample + file with this problem. + * Prevent buffer overrun when converting a password to an encryption key. Thanks to Gynvael Coldwind and Mateusz Jurczyk of the Google Security Team for providing a sample file with this @@ -1,6 +1,9 @@ 5.2.0 ===== + * Before release: remember to bump minor shared library version since + new methods were added (even though private). + * Figure out what about a3576a73593987b26cd3eff346f8f7c11f713cbd broke binary compatibility. diff --git a/include/qpdf/QPDF.hh b/include/qpdf/QPDF.hh index dfce40c1..71ff7e27 100644 --- a/include/qpdf/QPDF.hh +++ b/include/qpdf/QPDF.hh @@ -668,6 +668,9 @@ class QPDF void getAllPagesInternal(QPDFObjectHandle cur_pages, std::vector<QPDFObjectHandle>& result); + void getAllPagesInternal2(QPDFObjectHandle cur_pages, + std::vector<QPDFObjectHandle>& result, + std::set<QPDFObjGen>& visited); void insertPage(QPDFObjectHandle newpage, int pos); int findPage(QPDFObjGen const& og); int findPage(QPDFObjectHandle& page); @@ -1023,6 +1026,12 @@ class QPDF std::map<std::string, std::vector<QPDFObjectHandle> >&, std::vector<QPDFObjectHandle>& all_pages, bool allow_changes, bool warn_skipped_keys); + void pushInheritedAttributesToPageInternal2( + QPDFObjectHandle, + std::map<std::string, std::vector<QPDFObjectHandle> >&, + std::vector<QPDFObjectHandle>& all_pages, + bool allow_changes, bool warn_skipped_keys, + std::set<QPDFObjGen>& visited); void updateObjectMaps(ObjUser const& ou, QPDFObjectHandle oh); void updateObjectMapsInternal(ObjUser const& ou, QPDFObjectHandle oh, std::set<QPDFObjGen>& visited, bool top); 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 diff --git a/qpdf/qtest/qpdf.test b/qpdf/qtest/qpdf.test index 75726ca4..bd509264 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 += 75; +$n_tests += 76; $td->runtest("qpdf version", {$td->COMMAND => "qpdf --version"}, @@ -566,6 +566,10 @@ $td->runtest("ensure arguments to R are direct", {$td->COMMAND => "qpdf --check indirect-r-arg.pdf"}, {$td->FILE => "indirect-r-arg.out", $td->EXIT_STATUS => 2}, $td->NORMALIZE_NEWLINES); +$td->runtest("detect loops in pages structure", + {$td->COMMAND => "qpdf --check pages-loop.pdf"}, + {$td->FILE => "pages-loop.out", $td->EXIT_STATUS => 2}, + $td->NORMALIZE_NEWLINES); show_ntests(); # ---------- diff --git a/qpdf/qtest/qpdf/pages-loop.out b/qpdf/qtest/qpdf/pages-loop.out new file mode 100644 index 00000000..08643aa4 --- /dev/null +++ b/qpdf/qtest/qpdf/pages-loop.out @@ -0,0 +1,5 @@ +checking pages-loop.pdf +PDF Version: 1.3 +File is not encrypted +File is not linearized +pages-loop.pdf (object 3 0): Loop detected in /Pages structure (getAllPages) diff --git a/qpdf/qtest/qpdf/pages-loop.pdf b/qpdf/qtest/qpdf/pages-loop.pdf new file mode 100644 index 00000000..a41b7502 --- /dev/null +++ b/qpdf/qtest/qpdf/pages-loop.pdf @@ -0,0 +1,102 @@ +%PDF-1.3 +%¿÷¢þ +%QDF-1.0 + +%% Original object ID: 1 0 +1 0 obj +<< + /Pages 2 0 R + /Type /Catalog +>> +endobj + +%% Original object ID: 2 0 +2 0 obj +<< + /Count 1 + /Kids [ + 3 0 R + 2 0 R + ] + /Type /Pages +>> +endobj + +%% Page 1 +%% Original object ID: 3 0 +3 0 obj +<< + /Contents 4 0 R + /MediaBox [ + 0 + 0 + 612 + 792 + ] + /Parent 2 0 R + /Resources << + /Font << + /F1 6 0 R + >> + /ProcSet 7 0 R + >> + /Type /Page +>> +endobj + +%% Contents for page 1 +%% Original object ID: 4 0 +4 0 obj +<< + /Length 5 0 R +>> +stream +BT + /F1 24 Tf + 72 720 Td + (Potato) Tj +ET +endstream +endobj + +5 0 obj +44 +endobj + +%% Original object ID: 6 0 +6 0 obj +<< + /BaseFont /Helvetica + /Encoding /WinAnsiEncoding + /Name /F1 + /Subtype /Type1 + /Type /Font +>> +endobj + +%% Original object ID: 5 0 +7 0 obj +[ + /PDF + /Text +] +endobj + +xref +0 8 +0000000000 65535 f +0000000052 00000 n +0000000133 00000 n +0000000252 00000 n +0000000494 00000 n +0000000593 00000 n +0000000639 00000 n +0000000784 00000 n +trailer << + /Root 1 0 R + /Size 8 + /ID [<395875d4235973eebbade9c7e9e7f857><395875d4235973eebbade9c7e9e7f857>] +>> +startxref +819 +%%EOF |