Hi all, Please find enclosed a patch against SVN rev 1937 which fixes three important issues with PdfPagesTree::GetPageNode().
To demonstrate the issues the unit test PagesTreeTest has been extended by three new tests which all fail for r1937 and are fixed by this patch. The patch includes: 1) A real fix of CVE-2017-8054 (not really fixed upto r1937!) for handling of cyclic trees, see testCyclicTree() 2) A fix for handling of subtrees with „/Kids []“ and „/Count 0“ which is completely valid according to the PDF spec, see testEmptyKidsTree() 3) A changed behavior for trees with nested kids array which are not valid according to the PDF spec and now yield an NULL ptr, see testNestedArrayTree() Please note that this patch superseeds my former patch named „patch_getpagenode_cyclic_trees.diff“ against r1935, which only covered issue 1. I am looking forward to your feedback! Best regards, Amin
patch_getpagenode_rev1937.diff
Description: Binary data
> Am 22.08.2018 um 16:32 schrieb a.mas...@gmx.de: > > Hello again, > > Haven’t received any feedback on this issue, yet. So, I started to „dive" > into the code of PdfPagesTree::GetPageNode(). Now, I am even more concerned > that for the sake of correctness and security this function needs a rewrite > especially with the removal of GetPageNodeFromArray(). > > Please find enclosed a small patch against SVN rev 1935 for another problem > of GetPageNode(): It fixes a DoS vulnerability similar to CVE-2017-8054 which > may cause infinite recursion on cyclic trees. For clearity, I have also > extended the unit test. > > I am looking forward to your feedback! > > Best regards, > Amin > > <patch_getpagenode_cyclic_trees.diff> > >> Am 20.08.2018 um 16:29 schrieb A. Massad <a.mas...@gmx.de>: >> >> Hi Everyone, >> >> There is a problem with PdfPagesTree::GetPageNode() which yields NULL for >> valid PDFs. >> >> E.g. GetPageNode() for nPageNum=1 fails for this 3 page PDF: >> https://eur-lex.europa.eu/legal-content/DE/TXT/PDF/?uri=CELEX:52018XC0810(05)&from=DE >> >> This PDF is an example for a strange but valid page tree containing >> "/Pages“-Nodes with "/Count 0“ and „/Kids [ ]“. >> According to the PDF Spec "Section 7.7.3 Page Tree / 7.7.3.1 General" this >> tree should be handled: >> [...] >> Closer inspection of the code in GetPageNode() and GetPageNodeFromArray() >> shows that there is considerable code duplication and a lot of special >> cases, even for malformed PDFs. In fact, I would like to propose the >> complete removal of GetPageNodeFromArray() because it’s not needed, the >> condition for calling it is currently wrong and not easy to correct, and it >> introduces unclean code. There is another call to GetPageNodeFromArray() >> which also is unsure about its results and tries at least to correct this by >> checking the result for NULL. >> >> Rather the full tree traversal in GetPageNode() would be sufficient and >> correct for all cases. This end clearly needs further inspection of a PoDoFo >> expert. >> >> Best regards, >> Amin
signature.asc
Description: Message signed with OpenPGP
------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________ Podofo-users mailing list Podofo-users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/podofo-users
