Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
SUSE:SLE-12:Update
podofo.34526
r1941-Fix-CVE-2017-8054-and-other-issues-keepin...
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File r1941-Fix-CVE-2017-8054-and-other-issues-keeping-binary-compat.patch of Package podofo.34526
------------------------------------------------------------------------ r1941 | mc-zyx | 2018-09-30 16:56:19 +0200 (dom 30 de sep de 2018) | 10 líneas Patch by Amin Massad: Fix bugs in PdfPagesTree::GetPageNode() / PdfPagesTree::GetPageNodeFromArray() 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() Modified by Antonio Larrosa <alarrosa@suse.com> so the patch doesn't break binary compatibility Index: src/doc/PdfPagesTree.cpp =================================================================== --- src/doc/PdfPagesTree.cpp (revisión: 1940) +++ src/doc/PdfPagesTree.cpp (revisión: 1941) @@ -335,7 +335,6 @@ const PdfArray & rKidsArray = pObj->GetArray(); PdfArray::const_iterator it = rKidsArray.begin(); - const size_t numDirectKids = rKidsArray.size(); const size_t numKids = GetChildCount(pParent); // use <= since nPageNum is 0-based @@ -347,62 +346,25 @@ return NULL; } - //printf("Fetching: %i %i %i\n", numDirectKids, numKids, nPageNum ); - if( numDirectKids == numKids && static_cast<size_t>(nPageNum) < numDirectKids ) + //printf("Fetching: %i %i\n", numKids, nPageNum ); + + // We have to traverse the tree + // + // BEWARE: There is no valid shortcut for tree traversal. + // Even if eKidsArray.size()==numKids, this does not imply that + // eKidsArray can be accessed with the index of the page directly. + // The tree could have an arbitrary complex structure because + // internal nodes with no leaves (page objects) are not forbidden + // by the PDF spec. + while( it != rKidsArray.end() ) { - // This node has only page nodes as kids, - // so we can access the array directly - rLstParents.push_back( pParent ); - return GetPageNodeFromArray( nPageNum, rKidsArray, rLstParents ); - } - else - { - // We have to traverse the tree - while( it != rKidsArray.end() ) + if(!(*it).IsReference() ) { - if( (*it).IsArray() ) - { // Fixes PDFs broken by having trees with arrays nested once - - rLstParents.push_back( pParent ); + PdfError::LogMessage( eLogSeverity_Critical, "Requesting page index %i. Invalid datatype in kids array: %s\n", + nPageNum, (*it).GetDataTypeString()); + return NULL; + } - // the following code is to find the reference to log this with - const PdfReference & rIterArrayRef = (*it).Reference(); - PdfReference refToLog; - bool isDirectObject // don't worry about 0-num. indirect ones - = ( !(rIterArrayRef.ObjectNumber() ) ); - if ( isDirectObject ) - { - if ( !(pObj->Reference().ObjectNumber() ) ) // rKidsArray's - { - refToLog = pParent->Reference(); - } - else - { - refToLog = pObj->Reference(); - } - } - else - { - refToLog = rIterArrayRef; - } - PdfError::LogMessage( eLogSeverity_Error, - "Entry in Kids array is itself an array" - "%s reference: %s\n", isDirectObject ? " (direct object)" - ", in object with" : ",", refToLog.ToString().c_str() ); - - const PdfArray & rIterArray = (*it).GetArray(); - - // is the array large enough to potentially have the page? - if( static_cast<size_t>(nPageNum) < rIterArray.GetSize() ) - { - PdfObject* pPageNode = GetPageNodeFromArray( nPageNum, - rIterArray, rLstParents ); - if ( pPageNode ) // and if not, search further - return pPageNode; - } - } - else if( (*it).IsReference() ) - { PdfObject* pChild = GetRoot()->GetOwner()->GetObject( (*it).GetReference() ); if (!pChild) { @@ -416,13 +378,28 @@ int childCount = GetChildCount( pChild ); if( childCount < nPageNum + 1 ) // Pages are 0 based, but count is not { - // skip this page node - // and go to the next one + // skip this page tree node + // and go to the next child in rKidsArray nPageNum -= childCount; } else { + // page is in the subtree of pChild + // => call GetPageNode() recursively + rLstParents.push_back( pParent ); + + if ( std::find( rLstParents.begin(), rLstParents.end(), pChild ) + != rLstParents.end() ) // cycle in parent list detected, fend + { // off security vulnerability similar to CVE-2017-8054 (infinite recursion) + std::ostringstream oss; + oss << "Cycle in page tree: child in /Kids array of object " + << ( *(rLstParents.rbegin()) )->Reference().ToString() + << " back-references to object " << pChild->Reference() + .ToString() << " one of whose descendants the former is."; + PODOFO_RAISE_ERROR_INFO( ePdfError_PageNotFound, oss.str() ); + } + return this->GetPageNode( nPageNum, pChild, rLstParents ); } } @@ -430,6 +407,7 @@ { if( 0 == nPageNum ) { + // page found rLstParents.push_back( pParent ); return pChild; } @@ -448,24 +426,18 @@ "Invalid datatype referenced in kids array: %s\n" "Reference to invalid object: %i %i R\n", nPageNum, pChild->GetDataTypeString(), nLogObjNum, nLogGenNum); + return NULL; } - } - else - { - PdfError::LogMessage( eLogSeverity_Critical, "Requesting page index %i. Invalid datatype in kids array: %s\n", - nPageNum, (*it).GetDataTypeString()); - return NULL; - } ++it; } - } return NULL; } PdfObject* PdfPagesTree::GetPageNodeFromArray( int nPageNum, const PdfArray & rKidsArray, PdfObjectList & rLstParents ) { + PdfError::LogMessage( eLogSeverity_Warning, "PdfPagesTree::GetPageNodeFromArray is deprecated and will be removed soon"); if( static_cast<size_t>(nPageNum) >= rKidsArray.GetSize() ) { PdfError::LogMessage( eLogSeverity_Critical, "Requesting page index %i from array of size %i\n", Index: test/unit/PagesTreeTest.cpp =================================================================== --- test/unit/PagesTreeTest.cpp (revisión: 1940) +++ test/unit/PagesTreeTest.cpp (revisión: 1941) @@ -22,6 +22,8 @@ #include <podofo.h> +#include <sstream> + #define PODOFO_TEST_PAGE_KEY "PoDoFoTestPageNumber" #define PODOFO_TEST_NUM_PAGES 100 @@ -70,6 +72,58 @@ CPPUNIT_ASSERT_THROW( writer.GetPage( 1 ), PdfError ); } +void PagesTreeTest::testCyclicTree() +{ + for (int pass=0; pass < 2; pass++) + { + PdfMemDocument doc; + CreateCyclicTree( doc, pass==1); + //doc.Write(pass==0?"tree_valid.pdf":"tree_cyclic.pdf"); + for (int pagenum=0; pagenum < doc.GetPageCount(); pagenum++) + { + if (pass==0) + { + // pass 0: + // valid tree without cycles should yield all pages + PdfPage* pPage = doc.GetPage( pagenum ); + CPPUNIT_ASSERT_EQUAL( pPage != NULL, true ); + CPPUNIT_ASSERT_EQUAL( IsPageNumber( pPage, pagenum ), true ); + } + else + { + // pass 1: + // cyclic tree must throw exception to prevent infinite recursion + CPPUNIT_ASSERT_THROW( doc.GetPage( pagenum ), PdfError ); + } + } + } +} + +void PagesTreeTest::testEmptyKidsTree() +{ + PdfMemDocument doc; + CreateEmptyKidsTree(doc); + //doc.Write("tree_zerokids.pdf"); + for (int pagenum=0; pagenum < doc.GetPageCount(); pagenum++) + { + PdfPage* pPage = doc.GetPage( pagenum ); + CPPUNIT_ASSERT_EQUAL( pPage != NULL, true ); + CPPUNIT_ASSERT_EQUAL( IsPageNumber( pPage, pagenum ), true ); + } +} + +void PagesTreeTest::testNestedArrayTree() +{ + PdfMemDocument doc; + CreateNestedArrayTree(doc); + //doc.Write("tree_nested_array.pdf"); + for (int pagenum=0; pagenum < doc.GetPageCount(); pagenum++) + { + PdfPage* pPage = doc.GetPage( pagenum ); + CPPUNIT_ASSERT_EQUAL( pPage == NULL, true ); + } +} + void PagesTreeTest::testCreateDelete() { PdfMemDocument writer; @@ -354,7 +408,153 @@ pRoot->GetDictionary().AddKey( PdfName("Count"), static_cast<pdf_int64>(PODOFO_TEST_NUM_PAGES) ); } +std::vector<PdfPage*> PagesTreeTest::CreateSamplePages( PdfMemDocument & rDoc, + int nPageCount) +{ + PdfFont* pFont; + // create font + pFont = rDoc.CreateFont( "Arial" ); + if( !pFont ) + { + PODOFO_RAISE_ERROR( ePdfError_InvalidHandle ); + } + pFont->SetFontSize( 16.0 ); + + std::vector<PdfPage*> pPage(nPageCount); + for (int i = 0; i < nPageCount; ++i) + { + pPage[i] = new PdfPage( PdfPage::CreateStandardPageSize( ePdfPageSize_A4 ), + &(rDoc.GetObjects()) ); + pPage[i]->GetObject()->GetDictionary().AddKey( PODOFO_TEST_PAGE_KEY, + static_cast<pdf_int64>(i) ); + + PdfPainter painter; + painter.SetPage( pPage[i] ); + painter.SetFont( pFont ); + std::ostringstream os; + os << "Page " << i+1; + painter.DrawText( 200, 200, os.str() ); + painter.FinishPage(); + } + + return pPage; +} + +std::vector<PdfObject*> PagesTreeTest::CreateNodes( PdfMemDocument & rDoc, + int nNodeCount) +{ + std::vector<PdfObject*> pNode(nNodeCount); + + for (int i = 0; i < nNodeCount; ++i) + { + pNode[i]=rDoc.GetObjects().CreateObject("Pages"); + // init required keys + pNode[i]->GetDictionary().AddKey( "Kids", PdfArray()); + pNode[i]->GetDictionary().AddKey( "Count", PdfVariant(static_cast<pdf_int64>(0L))); + } + + return pNode; +} + +void PagesTreeTest::CreateCyclicTree( PoDoFo::PdfMemDocument & rDoc, + bool bCreateCycle ) +{ + const int COUNT = 3; + + std::vector<PdfPage*> pPage=CreateSamplePages( rDoc, COUNT ); + std::vector<PdfObject*> pNode=CreateNodes( rDoc, 2 ); + + // manually insert pages into pagetree + PdfObject* pRoot = rDoc.GetPagesTree()->GetObject(); + + // tree layout (for !bCreateCycle): + // + // root + // +-- node0 + // +-- node1 + // | +-- page0 + // | +-- page1 + // \-- page2 + + // root node + AppendChildNode(pRoot, pNode[0]); + + // tree node 0 + AppendChildNode(pNode[0], pNode[1]); + AppendChildNode(pNode[0], pPage[2]->GetObject()); + + // tree node 1 + AppendChildNode(pNode[1], pPage[0]->GetObject()); + AppendChildNode(pNode[1], pPage[1]->GetObject()); + + if (bCreateCycle) + { + // invalid tree: Cycle!!! + // was not detected in PdfPagesTree::GetPageNode() rev. 1937 + pNode[0]->GetIndirectKey("Kids")->GetArray()[0]=pRoot->Reference(); + } +} + +void PagesTreeTest::CreateEmptyKidsTree( PoDoFo::PdfMemDocument & rDoc ) +{ + const int COUNT = 3; + + std::vector<PdfPage*> pPage=CreateSamplePages( rDoc, COUNT ); + std::vector<PdfObject*> pNode=CreateNodes( rDoc, 3 ); + + // manually insert pages into pagetree + PdfObject* pRoot = rDoc.GetPagesTree()->GetObject(); + + // tree layout: + // + // root + // +-- node0 + // | +-- page0 + // | +-- page1 + // | +-- page2 + // +-- node1 + // \-- node2 + + // root node + AppendChildNode(pRoot, pNode[0]); + AppendChildNode(pRoot, pNode[1]); + AppendChildNode(pRoot, pNode[2]); + + // tree node 0 + AppendChildNode(pNode[0], pPage[0]->GetObject()); + AppendChildNode(pNode[0], pPage[1]->GetObject()); + AppendChildNode(pNode[0], pPage[2]->GetObject()); + + // tree node 1 and node 2 are left empty: this is completely valid + // according to the PDF spec, i.e. the required keys may have the + // values "/Kids [ ]" and "/Count 0" +} + +void PagesTreeTest::CreateNestedArrayTree( PoDoFo::PdfMemDocument & rDoc ) +{ + const int COUNT = 3; + + std::vector<PdfPage*> pPage=CreateSamplePages( rDoc, COUNT ); + PdfObject* pRoot = rDoc.GetPagesTree()->GetObject(); + + // create kids array + PdfArray kids; + for (int i=0; i < COUNT; i++) + { + kids.push_back( pPage[i]->GetObject()->Reference() ); + pPage[i]->GetObject()->GetDictionary().AddKey( PdfName("Parent"), pRoot->Reference()); + } + + // create nested kids array + PdfArray nested; + nested.push_back(kids); + + // manually insert pages into pagetree + pRoot->GetDictionary().AddKey( PdfName("Count"), static_cast<pdf_int64>(COUNT) ); + pRoot->GetDictionary().AddKey( PdfName("Kids"), nested); +} + bool PagesTreeTest::IsPageNumber( PoDoFo::PdfPage* pPage, int nNumber ) { # pdf_int64 lPageNumber = pPage->GetObject()->GetDictionary().GetKeyAsLong( PODOFO_TEST_PAGE_KEY, -1 ); long long lPageNumber = pPage->GetObject()->GetDictionary().GetKeyAsLong( PODOFO_TEST_PAGE_KEY, -1 ); @@ -367,3 +567,33 @@ else return true; } + +void PagesTreeTest::AppendChildNode(PdfObject* pParent, PdfObject* pChild) +{ + // 1. Add the reference of the new child to the kids array of pParent + PdfArray kids; + PdfObject* oldKids=pParent->GetIndirectKey("Kids"); + if (oldKids && oldKids->IsArray()) kids=oldKids->GetArray(); + kids.push_back(pChild->Reference()); + pParent->GetDictionary().AddKey( PdfName("Kids"), kids); + + // 2. If the child is a page (leaf node), increase count of every parent + // (which also includes pParent) + if( pChild->GetDictionary().GetKeyAsName( PdfName( "Type" ) ) + == PdfName( "Page" ) ) + { + PdfObject* node=pParent; + while (node) + { + pdf_int64 count=0; + if (node->GetIndirectKey("Count")) count=node->GetIndirectKey("Count")->GetNumber(); + count++; + node->GetDictionary().AddKey( PdfName("Count"), count); + + node=node->GetIndirectKey("Parent"); + } + } + + // 3. Add Parent key to the child + pChild->GetDictionary().AddKey( PdfName("Parent"), pParent->Reference()); +} Index: test/unit/PagesTreeTest.h =================================================================== --- test/unit/PagesTreeTest.h (revisión: 1940) +++ test/unit/PagesTreeTest.h (revisión: 1941) @@ -21,11 +21,14 @@ #ifndef _PAGES_TREE_TEST_H_ #define _PAGES_TREE_TEST_H_ +#include <vector> + #include <cppunit/extensions/HelperMacros.h> namespace PoDoFo { class PdfMemDocument; class PdfPage; +class PdfObject; }; /** This test tests the class PdfPagesTree @@ -35,6 +38,9 @@ CPPUNIT_TEST_SUITE( PagesTreeTest ); CPPUNIT_TEST( testEmptyTree ); CPPUNIT_TEST( testEmptyDoc ); + CPPUNIT_TEST( testCyclicTree ); + CPPUNIT_TEST( testEmptyKidsTree ); + CPPUNIT_TEST( testNestedArrayTree ); CPPUNIT_TEST( testCreateDelete ); CPPUNIT_TEST( testGetPagesCustom ); CPPUNIT_TEST( testGetPagesPoDoFo ); @@ -52,6 +58,9 @@ void testEmptyTree(); void testEmptyDoc(); + void testCyclicTree(); + void testEmptyKidsTree(); + void testNestedArrayTree(); void testCreateDelete(); void testGetPagesCustom(); void testGetPagesPoDoFo(); @@ -98,7 +107,58 @@ */ void CreateTestTreeCustom( PoDoFo::PdfMemDocument & rDoc ); + /** + * Create a pages tree with cycles to test prevention of endless + * recursion as mentioned in different CVE reports. + * + * \param bCreateCycle if true a cyclic tree is created, otherwise a + * valid tree without cycles + */ + void CreateCyclicTree( PoDoFo::PdfMemDocument & rDoc, + bool bCreateCycle ); + + /** + * Create a pages tree with nodes containing empty kids. + * + * This is completely valid according to the PDF spec, i.e. the + * required keys may have the values "/Kids [ ]" and "/Count 0" + * Such a tree must still be parsable by a conforming reader: + * + * <BLOCKQUOTE>The tree contains nodes of two types—intermediate + * nodes, called page tree nodes, and leaf nodes, called page + * objects—whose form is described in the subsequent subclauses. + * Conforming products shall be prepared to handle any form + * of tree structure built of such nodes.</BLOCKQUOTE> + */ + void CreateEmptyKidsTree( PoDoFo::PdfMemDocument & rDoc ); + + /** + * Ceate a pages tree with a nested kids array. + * + * Such a tree is not valid to the PDF spec, which requires they key + * "Kids" to be an array of indirect references. And the children shall + * only be page objects or other page tree nodes. + */ + void CreateNestedArrayTree( PoDoFo::PdfMemDocument & rDoc ); + + /** + * Create page object nodes (leaf nodes), + * where every page object has an additional + * key PoDoFoTestPageNumber with the original + * page number of the page. + */ + std::vector<PoDoFo::PdfPage*> CreateSamplePages( PoDoFo::PdfMemDocument & rDoc, + int nPageCount); + + /** + * Create page tree nodes (internal nodes) + */ + std::vector<PoDoFo::PdfObject*> CreateNodes( PoDoFo::PdfMemDocument & rDoc, + int nNodeCount); + bool IsPageNumber( PoDoFo::PdfPage* pPage, int nNumber ); + + void AppendChildNode(PoDoFo::PdfObject* pParent, PoDoFo::PdfObject* pChild); }; #endif // _PAGES_TREE_TEST_H_ ------------------------------------------------------------------------
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
