Commit afe0242b263a9e1a8d51dd81e42ab6de2e5127eb
1 parent
315092dd
Handle object ID 0 (fixes #99)
This is CVE-2017-9208. The QPDF library uses object ID 0 internally as a sentinel to represent a direct object, but prior to this fix, was not blocking handling of 0 0 obj or 0 0 R as a special case. Creating an object in the file with 0 0 obj could cause various infinite loops. The PDF spec doesn't allow for object 0. Having qpdf handle object 0 might be a better fix, but changing all the places in the code that assumes objid == 0 means direct would be risky.
Showing
9 changed files
with
116 additions
and
1 deletions
ChangeLog
| 1 | 2017-07-26 Jay Berkenbilt <ejb@ql.org> | 1 | 2017-07-26 Jay Berkenbilt <ejb@ql.org> |
| 2 | 2 | ||
| 3 | + * CVE-2017-9208: Handle references to and appearance of object 0 | ||
| 4 | + as a special case. Object 0 is not allowed, and qpdf was using it | ||
| 5 | + internally to represent direct objects. | ||
| 6 | + | ||
| 3 | * CVE-2017-9209: Fix infinite loop caused by attempting to | 7 | * CVE-2017-9209: Fix infinite loop caused by attempting to |
| 4 | reconstruct the xref table while already in the process of | 8 | reconstruct the xref table while already in the process of |
| 5 | reconstructing the xref table. | 9 | reconstructing the xref table. |
libqpdf/QPDF.cc
| @@ -1350,6 +1350,14 @@ QPDF::readObjectAtOffset(bool try_recovery, | @@ -1350,6 +1350,14 @@ QPDF::readObjectAtOffset(bool try_recovery, | ||
| 1350 | objid = atoi(tobjid.getValue().c_str()); | 1350 | objid = atoi(tobjid.getValue().c_str()); |
| 1351 | generation = atoi(tgen.getValue().c_str()); | 1351 | generation = atoi(tgen.getValue().c_str()); |
| 1352 | 1352 | ||
| 1353 | + if (objid == 0) | ||
| 1354 | + { | ||
| 1355 | + QTC::TC("qpdf", "QPDF object id 0"); | ||
| 1356 | + throw QPDFExc(qpdf_e_damaged_pdf, this->file->getName(), | ||
| 1357 | + this->last_object_description, offset, | ||
| 1358 | + "object with ID 0"); | ||
| 1359 | + } | ||
| 1360 | + | ||
| 1353 | if ((exp_objid >= 0) && | 1361 | if ((exp_objid >= 0) && |
| 1354 | (! ((objid == exp_objid) && (generation == exp_generation)))) | 1362 | (! ((objid == exp_objid) && (generation == exp_generation)))) |
| 1355 | { | 1363 | { |
libqpdf/QPDFObjectHandle.cc
| @@ -1089,6 +1089,16 @@ QPDFObjectHandle::parseInternal(PointerHolder<InputSource> input, | @@ -1089,6 +1089,16 @@ QPDFObjectHandle::parseInternal(PointerHolder<InputSource> input, | ||
| 1089 | QPDFObjectHandle | 1089 | QPDFObjectHandle |
| 1090 | QPDFObjectHandle::newIndirect(QPDF* qpdf, int objid, int generation) | 1090 | QPDFObjectHandle::newIndirect(QPDF* qpdf, int objid, int generation) |
| 1091 | { | 1091 | { |
| 1092 | + if (objid == 0) | ||
| 1093 | + { | ||
| 1094 | + // Special case: QPDF uses objid 0 as a sentinel for direct | ||
| 1095 | + // objects, and the PDF specification doesn't allow for object | ||
| 1096 | + // 0. Treat indirect references to object 0 as null so that we | ||
| 1097 | + // never create an indirect object with objid 0. | ||
| 1098 | + QTC::TC("qpdf", "QPDFObjectHandle indirect with 0 objid"); | ||
| 1099 | + return newNull(); | ||
| 1100 | + } | ||
| 1101 | + | ||
| 1092 | return QPDFObjectHandle(qpdf, objid, generation); | 1102 | return QPDFObjectHandle(qpdf, objid, generation); |
| 1093 | } | 1103 | } |
| 1094 | 1104 |
qpdf/qpdf.testcov
| @@ -273,3 +273,6 @@ QPDFWriter standard deterministic ID 1 | @@ -273,3 +273,6 @@ QPDFWriter standard deterministic ID 1 | ||
| 273 | QPDFWriter linearized deterministic ID 1 | 273 | QPDFWriter linearized deterministic ID 1 |
| 274 | QPDFWriter deterministic with no data 0 | 274 | QPDFWriter deterministic with no data 0 |
| 275 | qpdf-c called qpdf_set_deterministic_ID 0 | 275 | qpdf-c called qpdf_set_deterministic_ID 0 |
| 276 | +QPDFObjectHandle indirect with 0 objid 0 | ||
| 277 | +QPDF object id 0 0 | ||
| 278 | +QPDF caught recursive xref reconstruction 0 |
qpdf/qtest/qpdf.test
| @@ -206,7 +206,7 @@ $td->runtest("remove page we don't have", | @@ -206,7 +206,7 @@ $td->runtest("remove page we don't have", | ||
| 206 | show_ntests(); | 206 | show_ntests(); |
| 207 | # ---------- | 207 | # ---------- |
| 208 | $td->notify("--- Miscellaneous Tests ---"); | 208 | $td->notify("--- Miscellaneous Tests ---"); |
| 209 | -$n_tests += 79; | 209 | +$n_tests += 81; |
| 210 | 210 | ||
| 211 | $td->runtest("qpdf version", | 211 | $td->runtest("qpdf version", |
| 212 | {$td->COMMAND => "qpdf --version"}, | 212 | {$td->COMMAND => "qpdf --version"}, |
| @@ -220,6 +220,8 @@ $td->runtest("C API: qpdf version", | @@ -220,6 +220,8 @@ $td->runtest("C API: qpdf version", | ||
| 220 | 220 | ||
| 221 | # Files to reproduce various bugs | 221 | # Files to reproduce various bugs |
| 222 | foreach my $d ( | 222 | foreach my $d ( |
| 223 | + ["99", "object 0"], | ||
| 224 | + ["99b", "object 0"], | ||
| 223 | ["100","xref reconstruction loop"], | 225 | ["100","xref reconstruction loop"], |
| 224 | ["101", "resolve for exception text"], | 226 | ["101", "resolve for exception text"], |
| 225 | ) | 227 | ) |
qpdf/qtest/qpdf/issue-99.out
0 → 100644
qpdf/qtest/qpdf/issue-99.pdf
0 → 100644
No preview for this file type
qpdf/qtest/qpdf/issue-99b.out
0 → 100644
| 1 | +WARNING: issue-99b.pdf: file is damaged | ||
| 2 | +WARNING: issue-99b.pdf (object 1 0, file position 9): object with ID 0 | ||
| 3 | +WARNING: issue-99b.pdf: Attempting to reconstruct cross-reference table | ||
| 4 | +WARNING: issue-99b.pdf: object 1 0 not found in file after regenerating cross reference table | ||
| 5 | +operation for Dictionary object attempted on object of wrong type |
qpdf/qtest/qpdf/issue-99b.pdf
0 → 100644
| 1 | +%PDF-1.3 | ||
| 2 | +0 0 obj | ||
| 3 | +<< | ||
| 4 | + /Type /Catalog | ||
| 5 | + /Pages 2 0 R | ||
| 6 | +>> | ||
| 7 | +endobj | ||
| 8 | + | ||
| 9 | +2 0 obj | ||
| 10 | +<< | ||
| 11 | + /Type /Pages | ||
| 12 | + /Kids [ | ||
| 13 | + 3 0 R | ||
| 14 | + ] | ||
| 15 | + /Count 1 | ||
| 16 | +>> | ||
| 17 | +endobj | ||
| 18 | + | ||
| 19 | +3 0 obj | ||
| 20 | +<< | ||
| 21 | + /Type /Page | ||
| 22 | + /Parent 2 0 R | ||
| 23 | + /MediaBox [0 0 612 792] | ||
| 24 | + /Contents 4 0 R | ||
| 25 | + /Resources << | ||
| 26 | + /ProcSet 5 0 R | ||
| 27 | + /Font << | ||
| 28 | + /F1 6 0 R | ||
| 29 | + >> | ||
| 30 | + >> | ||
| 31 | +>> | ||
| 32 | +endobj | ||
| 33 | + | ||
| 34 | +4 0 obj | ||
| 35 | +<< | ||
| 36 | + /Length 44 | ||
| 37 | +>> | ||
| 38 | +stream | ||
| 39 | +BT | ||
| 40 | + /F1 24 Tf | ||
| 41 | + 72 720 Td | ||
| 42 | + (Potato) Tj | ||
| 43 | +ET | ||
| 44 | +endstream | ||
| 45 | +endobj | ||
| 46 | + | ||
| 47 | +5 0 obj | ||
| 48 | +[ | ||
| 49 | |||
| 50 | + /Text | ||
| 51 | +] | ||
| 52 | +endobj | ||
| 53 | + | ||
| 54 | +6 0 obj | ||
| 55 | +<< | ||
| 56 | + /Type /Font | ||
| 57 | + /Subtype /Type1 | ||
| 58 | + /Name /F1 | ||
| 59 | + /BaseFont /Helvetica | ||
| 60 | + /Encoding /WinAnsiEncoding | ||
| 61 | +>> | ||
| 62 | +endobj | ||
| 63 | + | ||
| 64 | +xref | ||
| 65 | +0 7 | ||
| 66 | +0000000000 65535 f | ||
| 67 | +0000000009 00000 n | ||
| 68 | +0000000063 00000 n | ||
| 69 | +0000000135 00000 n | ||
| 70 | +0000000307 00000 n | ||
| 71 | +0000000403 00000 n | ||
| 72 | +0000000438 00000 n | ||
| 73 | +trailer << | ||
| 74 | + /Size 7 | ||
| 75 | + /Root 1 0 R | ||
| 76 | +>> | ||
| 77 | +startxref | ||
| 78 | +556 | ||
| 79 | +%%EOF |