Hello Mattia,
> Mattia Rizzolo has written on 21 December 2017 at 22:54:
>
>
> Control: tag -1 patch
>
> On Thu, Dec 21, 2017 at 04:55:00PM +0100, Matthias Brinke wrote:
>> 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
>> ... snip ...
>
> I could, but could you please send it upstream?
> The only way to interact with upstream is the podofo-users ML
> https://sourceforge.net/p/podofo/mailman/podofo-users/
There is a post on the podofo-users mailing list by zyx which
says that running test/unit/podofo-test a heap-use-after-free
bug was detected [1]. I've investigated that and implemented a
correction, which I tested with -fsanitize=address (ASan) in
a Debian sid chroot (up-to-date, mostly? minimal) through
sbuild (from jessie-backports), attached here. Only memory leaks
were detected, therefore the test log is not attached here, also
because it's 292 KiB large (many leaks, but that's OT here). As
far as I've already seen, the leaks seems to be elsewhere.
This patch is complete to add to the Debian package, so please
disregard the older (and incorrect) patch in this bug thread.
I'm sorry I'm answering only now, the divergence between the
package and the upstream svn repository (also through the partial
revert) presented some challenges in patch handling. Because of
this, the patch is also not for forwarding (it wouldn't apply there).
I'm not changing the fixed-upstream tag as the partial revert is
already mentioned in the security-tracker notes and would've been
reflected in the (removal of the) tag if it was necessary, right?
>
> --
> regards,
> Mattia Rizzolo
>
Best regards, Matthias Brinke
[1] https://sourceforge.net/p/podofo/mailman/message/36215307/
Description: Fix CVE-2017-8054, also avoiding use-after-free
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: not-needed
Last-Update: 2018-02-05
---
Index: src/doc/PdfPagesTree.cpp
===================================================================
--- 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,18 @@
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;
+ }
+ PdfVariant rVarFirstEntry = rVarArray[0]; // avoids use-after-free
+ rVar = rVarFirstEntry; // in this line (rVar-ref'd array is freed)
+ continue;
}
else if( !rVar.IsReference() )
{
@@ -502,6 +516,18 @@
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.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 );
--- libpodofo-0.9.5.orig/src/base/PdfError.h
+++ libpodofo-0.9.5/src/base/PdfError.h
@@ -53,10 +53,10 @@
namespace PoDoFo {
-/** Error Code defines which are used in PdfError to describe the error.
+/** Error Code enum values which are used in PdfError to describe the error.
*
- * If you add an error code to this enum, please also add it to PdfError::ErrorName()
- * and PdfError::ErrorMessage().
+ * If you add an error code to this enum, please also add it
+ * to PdfError::ErrorName() and PdfError::ErrorMessage().
*
* \see PdfError
*/
@@ -149,16 +149,18 @@ enum ELogSeverity {
/** \def PODOFO_RAISE_ERROR( x )
*
- * Set the value of the variable eCode (which has to exist in the current function) to x
- * and return the eCode.
+ * Throw an exception of type PdfError with the error code x, which should be
+ * one of the values of the enum EPdfError. File and line info are included.
*/
#define PODOFO_RAISE_ERROR( x ) throw ::PoDoFo::PdfError( x, __FILE__, __LINE__ );
/** \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.
+ * Throw an exception of type PdfError with the error code x, which should be
+ * one of the values of the enum EPdfError. File and line info are included.
+ * Additionally extra information on the error, y is set, which will also be
+ * output by PdfError::PrintErrorMsg().
+ * 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 );
@@ -183,6 +185,7 @@ class PODOFO_API PdfErrorInfo {
public:
PdfErrorInfo();
PdfErrorInfo( int line, const char* pszFile, const char* pszInfo );
+ PdfErrorInfo( int line, const char* pszFile, std::string sInfo );
PdfErrorInfo( int line, const char* pszFile, const wchar_t* pszInfo );
PdfErrorInfo( const PdfErrorInfo & rhs );
@@ -194,6 +197,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 sInfo ) { m_sInfo = sInfo; }
inline void SetInformation( const wchar_t* pszInfo ) { m_swInfo = pszInfo ? pszInfo : L""; }
private:
@@ -213,15 +217,15 @@ typedef TDequeErrorInfo::const_iterator
// Without this define doxygen thinks we have a class called PODOFO_EXCEPTION_API(PODOFO_API) ...
#define PODOFO_EXCEPTION_API_DOXYGEN PODOFO_EXCEPTION_API(PODOFO_API)
-/** The error handling class of PoDoFo lib.
- * Whenever a function encounters an error
- * a PdfError object is returned.
+/** The error handling class of the PoDoFo library.
+ * If a method encounters an error,
+ * a PdfError object is thrown as a C++ exception.
*
- * A PdfError with Error() == ErrOk means
- * successfull execution.
+ * This class does not inherit from std::exception.
*
- * This class provides also meaningfull
- * error descriptions.
+ * This class also provides meaningful error descriptions
+ * for the error codes which are values of the enum EPdfError,
+ * which are all codes PoDoFo uses (except the first and last one).
*/
class PODOFO_EXCEPTION_API_DOXYGEN PdfError {
public:
@@ -235,13 +239,13 @@ class PODOFO_EXCEPTION_API_DOXYGEN PdfEr
virtual void LogMessage( ELogSeverity eLogSeverity, const wchar_t* pszPrefix, const wchar_t* pszMsg, va_list & args ) = 0;
};
- /** Set a global static LogMessageCallback functor to repleace stderr output in LogMessageInternal
+ /** Set a global static LogMessageCallback functor to replace stderr output in LogMessageInternal.
* \param fLogMessageCallback the pointer to the new callback functor object
* \returns the pointer to the previous callback functor object
*/
static LogMessageCallback* SetLogMessageCallback(LogMessageCallback* fLogMessageCallback);
- /** Create a PdfError object initialized to ErrOk
+ /** Create a PdfError object initialized to ePdfError_ErrOk.
*/
PdfError();
@@ -251,12 +255,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
*/
@@ -276,37 +290,39 @@ class PODOFO_EXCEPTION_API_DOXYGEN PdfEr
*/
const PdfError & operator=( const EPdfError & eCode );
- /** Comparison operator compares 2 PdfError objects
+ /** Comparison operator, compares 2 PdfError objects
* \param rhs another PdfError object
* \returns true if both objects have the same error code.
*/
bool operator==( const PdfError & rhs );
- /** Overloaded comparison operator compares 2 PdfError objects
- * \param eCode an erroce code
+ /** Overloaded comparison operator, compares this PdfError object
+ * with an error code
+ * \param eCode an error code (value of the enum EPdfError)
* \returns true if this object has the same error code.
*/
bool operator==( const EPdfError & eCode );
- /** Comparison operator compares 2 PdfError objects
+ /** Comparison operator, compares 2 PdfError objects
* \param rhs another PdfError object
- * \returns true if both objects have the different error code.
+ * \returns true if the objects have different error codes.
*/
bool operator!=( const PdfError & rhs );
- /** Overloaded comparison operator compares 2 PdfError objects
- * \param eCode an erroce code
- * \returns true if this object has different error code.
+ /** Overloaded comparison operator, compares this PdfError object
+ * with an error code
+ * \param eCode an error code (value of the enum EPdfError)
+ * \returns true if this object has a different error code.
*/
bool operator!=( const EPdfError & eCode );
- /** Return the error code of this object
+ /** Return the error code of this object.
* \returns the error code of this object
*/
inline EPdfError GetError() const;
- /** Get access to the internal callstack of this error
- * \return the callstack
+ /** Get access to the internal callstack of this error.
+ * \returns the callstack deque of PdfErrorInfo objects.
*/
inline const TDequeErrorInfo & GetCallstack() const;
@@ -318,21 +334,36 @@ 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 pszInformation additional information on the error.
+ * \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.
*/
inline void SetError( const EPdfError & eCode, const char* pszFile = NULL, int line = 0, const char* pszInformation = NULL );
- /** Set additional error informatiom
- * \param pszInformation additional information on the error.
+ /** Set additional error information.
+ * \param pszInformation additional information on the error,
* e.g. how to fix the error. This string is intended to
* be shown to the user.
*/
inline void SetErrorInformation( const char* pszInformation );
- /** Set additional error informatiom
- * \param pszInformation additional information on the error.
+ /** Set additional error information.
+ * \param pszInformation additional information on the error,
* e.g. how to fix the error. This string is intended to
* be shown to the user.
*/
@@ -347,23 +378,39 @@ 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 pszInformation additional information on the error.
+ * \param pszInformation 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 = 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
+ * and false if the error code is ePdfError_ErrOk.
*/
inline bool IsError() const;
- /** Print an error message to stderr
+ /** Print an error message to stderr. This includes callstack
+ * and extra info, if any of either was set.
*/
void PrintErrorMsg() const;
/** Obtain error description.
- * \returns a c string describing the error.
+ * \returns a C string describing the error.
*/
const char* what() const;
@@ -392,7 +439,7 @@ class PODOFO_EXCEPTION_API_DOXYGEN PdfEr
*/
static void LogMessage( ELogSeverity eLogSeverity, const wchar_t* pszMsg, ... );
- /** Enable or disable Logging
+ /** Enable or disable logging.
* \param bEnable enable (true) or disable (false)
*/
static void EnableLogging( bool bEnable );
@@ -401,12 +448,12 @@ class PODOFO_EXCEPTION_API_DOXYGEN PdfEr
*/
static bool LoggingEnabled();
- /** Log a message to the logging system defined for PoDoFo for debugging
+ /** Log a message to the logging system defined for PoDoFo for debugging.
* \param pszMsg the message to be logged
*/
static void DebugMessage( const char* pszMsg, ... );
- /** Enable or disable the display of debugging messages
+ /** Enable or disable the display of debugging messages.
* \param bEnable enable (true) or disable (false)
*/
static void EnableDebug( bool bEnable );
@@ -487,6 +534,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() )
