Dear maintainer, hello all,
I have simplified my fix for CVE-2017-8054 (stack overflow
by infinite recursion from loop in pages tree) and tested
it again in an unstable chroot (entered by sbuild from
jessie-backports), which was clean, up-to-date and maybe
also minimal (why does it contain GNU autotools?). It is,
of course, attached here. The source for a generator just
using PoDoFo outputting the simple/mostly-minimal PoC I
tested with is also attached. Please accept it for your
next upload (could you please do it before upstream puts
out its next release or rc tarball?) and bump the shlibs
version, too, because a PdfError constructor is added by it.
Best regards, Matthias
Description: Fix CVE-2017-8054
The recursive call in the case of the requested page index
having an array has been removed to prevent possibility of
stack overflow there, and replaced it by code setting the
variable which had been modified in the new stack frame in
the former revision, directly, after checking its validity.
The second hunk inserts cycle detection for the parent list.
Author: Matthias Brinke <podofo-sec-cont...@mailbox.org>
Bug-Debian: https://bugs.debian.org/860995
Forwarded: no
Last-Update: 2017-12-21
---
--- libpodofo-0.9.5.orig/src/doc/PdfPagesTree.cpp
+++ libpodofo-0.9.5/src/doc/PdfPagesTree.cpp
@@ -34,6 +34,7 @@
#include "PdfPagesTree.h"
#include "base/PdfDefinesPrivate.h"
+#include <algorithm>
#include "base/PdfArray.h"
#include "base/PdfDictionary.h"
@@ -478,7 +479,17 @@ PdfObject* PdfPagesTree::GetPageNodeFrom
if( rVar.IsArray() )
{
// Fixes some broken PDFs who have trees with 1 element kids arrays
- return GetPageNodeFromArray( 0, rVar.GetArray(), rLstParents );
+ // Recursive call removed to prevent stack overflow, replaced by:
+ // all the following inside this conditional, plus restart looping
+ const PdfArray & rVarArray = rVar.GetArray();
+ if (rVarArray.GetSize() == 0)
+ {
+ PdfError::LogMessage( eLogSeverity_Critical, "Trying to access"
+ " first page index of empty array" );
+ return NULL;
+ }
+ rVar = rVarArray[0];
+ continue;
}
else if( !rVar.IsReference() )
{
@@ -502,6 +513,18 @@ PdfObject* PdfPagesTree::GetPageNodeFrom
if( !pgObject->GetDictionary().HasKey( "Kids" ) )
return NULL;
+ if ( std::find( rLstParents.begin(), rLstParents.end(), pgObject )
+ != rLstParents.end() ) // cycle in parent list detected, fend
+ { // off security vulnerability 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 " << pgObject->Reference()
+ .ToString() << " one of whose descendants the former is.";
+
+ PODOFO_RAISE_ERROR_INFO( ePdfError_PageNotFound, oss.str() );
+ }
+
rLstParents.push_back( pgObject );
rVar = *(pgObject->GetDictionary().GetKey( "Kids" ));
} else {
--- libpodofo-0.9.5.orig/src/base/PdfError.h
+++ libpodofo-0.9.5/src/base/PdfError.h
@@ -158,8 +158,8 @@ enum ELogSeverity {
/** \def PODOFO_RAISE_ERROR_INFO( x, y )
*
* Set the value of the variable eCode (which has to exist in the current function) to x
- * and return the eCode. Additionally additional information on the error y is set. y has
- * to be an c-string.
+ * and return the eCode. Additionally additional information on the error y is set.
+ * y can be a C string, but can also be a C++ std::string.
*/
#define PODOFO_RAISE_ERROR_INFO( x, y ) throw ::PoDoFo::PdfError( x, __FILE__, __LINE__, y );
@@ -184,6 +184,7 @@ class PODOFO_API PdfErrorInfo {
public:
PdfErrorInfo();
PdfErrorInfo( int line, const char* pszFile, const char* pszInfo );
+ PdfErrorInfo( int line, const char* pszFile, std::string pszInfo );
PdfErrorInfo( int line, const char* pszFile, const wchar_t* pszInfo );
PdfErrorInfo( const PdfErrorInfo & rhs );
@@ -195,6 +196,7 @@ class PODOFO_API PdfErrorInfo {
inline const std::wstring & GetInformationW() const { return m_swInfo; }
inline void SetInformation( const char* pszInfo ) { m_sInfo = pszInfo ? pszInfo : ""; }
+ inline void SetInformation( std::string pszInfo ) { m_sInfo = pszInfo; }
inline void SetInformation( const wchar_t* pszInfo ) { m_swInfo = pszInfo ? pszInfo : L""; }
private:
@@ -252,12 +254,22 @@ class PODOFO_EXCEPTION_API_DOXYGEN PdfEr
* Use the compiler macro __FILE__ to initialize the field.
* \param line the line in which the error has occured.
* Use the compiler macro __LINE__ to initialize the field.
- * \param pszInformation additional information on this error which mayy
- * be formatted like printf
+ * \param pszInformation additional information on this error
*/
PdfError( const EPdfError & eCode, const char* pszFile = NULL, int line = 0,
const char* pszInformation = NULL );
+ /** Create a PdfError object with a given error code.
+ * \param eCode the error code of this object
+ * \param pszFile the file in which the error has occured.
+ * Use the compiler macro __FILE__ to initialize the field.
+ * \param line the line in which the error has occured.
+ * Use the compiler macro __LINE__ to initialize the field.
+ * \param sInformation additional information on this error
+ */
+ explicit PdfError( const EPdfError & eCode, const char* pszFile, int line,
+ std::string sInformation );
+
/** Copy constructor
* \param rhs copy the contents of rhs into this object
*/
@@ -319,6 +331,21 @@ class PODOFO_EXCEPTION_API_DOXYGEN PdfEr
* \param line the line of source causing the error
* or 0. Typically you will use the gcc
* macro __LINE__ here.
+ * \param sInformation additional information on the error.
+ * e.g. how to fix the error. This string is intended to
+ * be shown to the user.
+ */
+ inline void SetError( const EPdfError & eCode, const char* pszFile, int line,
+ std::string sInformation );
+
+ /** Set the error code of this object.
+ * \param eCode the error code of this object
+ * \param pszFile the filename of the source file causing
+ * the error or NULL. Typically you will use
+ * the gcc macro __FILE__ here.
+ * \param line the line of source causing the error
+ * or 0. Typically you will use the gcc
+ * macro __LINE__ here.
* \param pszInformation additional information on the error.
* e.g. how to fix the error. This string is intended to
* be shown to the user.
@@ -354,6 +381,21 @@ class PODOFO_EXCEPTION_API_DOXYGEN PdfEr
*/
inline void AddToCallstack( const char* pszFile = NULL, int line = 0, const char* pszInformation = NULL );
+ /** Add callstack information to an error object. Always call this function
+ * if you get an error object but do not handle the error but throw it again.
+ *
+ * \param pszFile the filename of the source file causing
+ * the error or NULL. Typically you will use
+ * the gcc macro __FILE__ here.
+ * \param line the line of source causing the error
+ * or 0. Typically you will use the gcc
+ * macro __LINE__ here.
+ * \param sInformation additional information on the error.
+ * e.g. how to fix the error. This string is intended to
+ * be shown to the user.
+ */
+ inline void AddToCallstack( const char* pszFile, int line, std::string sInformation );
+
/** \returns true if an error code was set
* and false if the error code is ePdfError_ErrOk
*/
@@ -488,6 +530,22 @@ void PdfError::AddToCallstack( const cha
// -----------------------------------------------------
//
// -----------------------------------------------------
+void PdfError::SetError( const EPdfError & eCode, const char* pszFile, int line, std::string sInformation )
+{
+ m_error = eCode;
+ this->AddToCallstack( pszFile, line, sInformation );
+}
+
+// -----------------------------------------------------
+//
+// -----------------------------------------------------
+void PdfError::AddToCallstack( const char* pszFile, int line, std::string sInformation )
+{
+ m_callStack.push_front( PdfErrorInfo( line, pszFile, sInformation ) );
+}
+// -----------------------------------------------------
+//
+// -----------------------------------------------------
void PdfError::SetErrorInformation( const char* pszInformation )
{
if( m_callStack.size() )
--- libpodofo-0.9.5.orig/src/base/PdfError.cpp
+++ libpodofo-0.9.5/src/base/PdfError.cpp
@@ -60,6 +60,12 @@ PdfErrorInfo::PdfErrorInfo()
{
}
+PdfErrorInfo::PdfErrorInfo( int line, const char* pszFile, std::string sInfo )
+ : m_nLine( line ), m_sFile( pszFile ? pszFile : "" ), m_sInfo( sInfo )
+{
+
+}
+
PdfErrorInfo::PdfErrorInfo( int line, const char* pszFile, const char* pszInfo )
: m_nLine( line ), m_sFile( pszFile ? pszFile : "" ), m_sInfo( pszInfo ? pszInfo : "" )
{
@@ -96,6 +102,12 @@ PdfError::PdfError()
}
PdfError::PdfError( const EPdfError & eCode, const char* pszFile, int line,
+ std::string sInformation )
+{
+ this->SetError( eCode, pszFile, line, sInformation );
+}
+
+PdfError::PdfError( const EPdfError & eCode, const char* pszFile, int line,
const char* pszInformation )
{
this->SetError( eCode, pszFile, line, pszInformation );
#include <podofo/podofo.h>
using namespace PoDoFo;
void CreatePagesLoopPoC(PdfDocument& doc)
{
PdfPage* pFirstPage = doc.GetPage( 0 );
PdfObject* pFirstPageObject = pFirstPage->GetObject();
PdfObject* pParentObject = pFirstPageObject->MustGetIndirectKey("Parent");
PdfArray & rParentKidsArray = pParentObject->MustGetIndirectKey("Kids")
->GetArray();
rParentKidsArray.insert(rParentKidsArray.begin(),
pParentObject->Reference());
pdf_int64 ll2 = 2; // needed to make the following call unambiguous
pParentObject->GetDictionary().AddKey("Count", ll2); // 1 original, 1 loop
}
#define FONTNAME "Helvetica"
#define TEXTPOS_X 72.0
#define TEXTPOS_Y 72.0
#define SAMPLE_TEXT "Sample"
void DrawSampleTextOnPage(PdfPage* pPage)
{
PdfPainter painter;
painter.SetPage( pPage );
PdfFont* pFont = PdfFontFactory::CreateBase14Font( FONTNAME,
ePdfFont_Normal,
PdfEncodingFactory::GlobalWinAnsiEncodingInstance(),
pPage->GetObject()->GetOwner() );
painter.SetFont( pFont );
painter.DrawText( TEXTPOS_X, TEXTPOS_Y, PdfString( SAMPLE_TEXT,
pFont->GetEncoding() ));
painter.FinishPage();
delete pFont;
}
#define OUTFILE "my-CVE-2017-8054-PoC.pdf"
int main()
{
PdfMemDocument doc;
PdfPage* pPage
= doc.CreatePage( PdfPage::CreateStandardPageSize( ePdfPageSize_A4 ));
DrawSampleTextOnPage( pPage );
CreatePagesLoopPoC( doc );
doc.Write( OUTFILE );
return 0;
}
