From 603f222365252f1a1e20303b3dbe52466be3053b Mon Sep 17 00:00:00 2001 From: Jay Berkenbilt Date: Tue, 25 Jul 2017 10:13:30 -0400 Subject: [PATCH] Fix infinite loop while reporting an error (fixes #101) This is CVE-2017-9210. The description string for an error message included unparsing an object, which is too complex of a thing to try to do while throwing an exception. There was only one example of this in the entire codebase, so it is not a pervasive problem. Fixing this eliminated one class of infinite loop errors. --- ChangeLog | 5 +++++ libqpdf/QPDFObjectHandle.cc | 3 +-- qpdf/qtest/qpdf.test | 16 +++++++++++++++- qpdf/qtest/qpdf/issue-101.out | 6 ++++++ qpdf/qtest/qpdf/issue-101.pdf | Bin 0 -> 4613 bytes 5 files changed, 27 insertions(+), 3 deletions(-) create mode 100644 qpdf/qtest/qpdf/issue-101.out create mode 100644 qpdf/qtest/qpdf/issue-101.pdf diff --git a/ChangeLog b/ChangeLog index 548106ee..32bafad6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +2017-07-26 Jay Berkenbilt + + * CVE-2017-9210: Fix infinite loop caused by attempting to unparse + an object for inclusion in the text of an exception. + 2015-11-10 Jay Berkenbilt * 6.0.0: release diff --git a/libqpdf/QPDFObjectHandle.cc b/libqpdf/QPDFObjectHandle.cc index 64a4e3c3..687ba439 100644 --- a/libqpdf/QPDFObjectHandle.cc +++ b/libqpdf/QPDFObjectHandle.cc @@ -1076,8 +1076,7 @@ QPDFObjectHandle::parseInternal(PointerHolder input, throw QPDFExc( qpdf_e_damaged_pdf, input->getName(), object_description, offset, - std::string("dictionary key not name (") + - key_obj.unparse() + ")"); + std::string("dictionary key is not not a name token")); } dict[key_obj.getName()] = val; } diff --git a/qpdf/qtest/qpdf.test b/qpdf/qtest/qpdf.test index c17271d9..e0b2609a 100644 --- a/qpdf/qtest/qpdf.test +++ b/qpdf/qtest/qpdf.test @@ -206,7 +206,7 @@ $td->runtest("remove page we don't have", show_ntests(); # ---------- $td->notify("--- Miscellaneous Tests ---"); -$n_tests += 77; +$n_tests += 78; $td->runtest("qpdf version", {$td->COMMAND => "qpdf --version"}, @@ -218,6 +218,20 @@ $td->runtest("C API: qpdf version", $td->EXIT_STATUS => 0}, $td->NORMALIZE_NEWLINES); +# Files to reproduce various bugs +foreach my $d ( + ["101", "resolve for exception text"], + ) +{ + my ($n, $description) = @$d; + $td->runtest($description, + {$td->COMMAND => "qpdf issue-$n.pdf a.pdf"}, + {$td->FILE => "issue-$n.out", + $td->EXIT_STATUS => 2}, + $td->NORMALIZE_NEWLINES); +} + + foreach (my $i = 1; $i <= 3; ++$i) { $td->runtest("misc tests", diff --git a/qpdf/qtest/qpdf/issue-101.out b/qpdf/qtest/qpdf/issue-101.out new file mode 100644 index 00000000..59bd8103 --- /dev/null +++ b/qpdf/qtest/qpdf/issue-101.out @@ -0,0 +1,6 @@ +WARNING: issue-101.pdf: file is damaged +WARNING: issue-101.pdf (file position 3526): xref not found +WARNING: issue-101.pdf: Attempting to reconstruct cross-reference table +WARNING: issue-101.pdf (object 5 0, file position 1509): attempting to recover stream length +WARNING: issue-101.pdf (object 5 0, file position 2097): attempting to recover stream length +issue-101.pdf (trailer, file position 2928): unknown token while reading object (ÿ) diff --git a/qpdf/qtest/qpdf/issue-101.pdf b/qpdf/qtest/qpdf/issue-101.pdf new file mode 100644 index 0000000000000000000000000000000000000000..801b7c3a9702f06311f987914f30e61b9cfa65bf GIT binary patch literal 4613 zcmc&%4OA4@6C}ku3 zun=LXX4c1*d+n?bD;2^(ChKt7^qf17-pfkuP% zIA}T$wIJmM+;&p8Ssx;UeMLlawlWW1P+(~l7^0Q#a`?nSGrcurmP6{40Vb5n0X86M zHr`gk6^gcS0z8G}E-Yg`DR$n)dGhFdGbXu9t=LmuQUa&G9B(P%yp}SXot1()&ugnf z#8^$ifb*~tex;~#S3|vg$90@Ovg4teSs~%`8^fY;NO<|!@Vc4}3+9GK){YH7Qv1a@ z&(+e;r>YZQeqn6uhOtGRzZ>QGI@R#`ms>wt+ag<;zbfxSecErWQFZ^GvvlOKR-F=?{3xpV<+^e z{;gwKy|e(=({)NX)g&In7ho@BHzUPmedLCOj{HYu2VW<9!`FPn%~>Yw8)@^qKaN@gH{U z_G)u3I2XMVn|xq;ZQb6N&YgWdzgKr~T*SrAGs?^Oo#qE+#V*;qUr#vSxb6K}D`UIT z7R+~E-Pcml%r)or^!9#sv*N?G-D}=>l3SZtHSt)hrB(gGiWOxQE7sK}M|@QAO+xee z!_6C8YoZR0I@~-ubm|Wr57(AFQg_zUiyw^$2c*}uG6%-kyTW7{iKQ?8<@BvzJkW9Tc0|X=_6Q19AMa&t zrG21{69M%wpj6X0NJWAxK$Wu4nW!IqRB=>ENS8*(#slEv3KpOoDh4PCbKW67PIZYg zf>HH-S|bhVbOK)jrAUd0g=b51aDoDnezQniVt)Thu7(e!0WjXj15^=ZAp@L?F`x2* zQGrszYcL)U4}I8*hys|EmMuEwo{A`E1lG>%H|d16X^}z!7f%clE|_;B8aaev9K*jh zR|)H-8fa4vk_1%3 zB4XkdgRw}l2UV*?<{&0OM+#sCCD0R8Jt2}{brDU!Kb;C40+fR_zdDP!fUpY|0U9*j z5-xN$NRnbh04@;9#R{YoF@ey?U-R3SUKJ}trI&T0I{>MGRA4v$U!>y3q?H@_A2mcm#$S8c3o{a zlpmV+_`Y=1wxsFFGZkYZ_`jYzee}e-mS=MxV=9`~JpIhBH@&BpMpY$NG<9;TL8Zm(Z{~Ex*)rpuOjitR5m6*bUA`C@- z6D{t6=%W%bK$XBFN|^vfLIQ+DRWfM&FajCy0INe~_XkQ(fK1AF{;gp*I$kT^81c>W ze_@niZIyFtLx>rGHa&Hep+6zC#1ejekmvF=;ou3iG8iRuiXu7bhjy_ zE$~P~+4S000|oSTSfC6>r5gGq zp`V5N2K=WHfIs18g!eE_>XF4OeZODhg_a#F)x^wku)h}e$k6+FoE%mPn|io0!WLL( zOx4E4s}nRP4P#18(Q8y{6{Am8n2aW6szzy~o1uG7p_>q6c_}80o?(