#4 CVE-2018-5783 - uncontrolled memory allocation in the PoDoFo::PdfVecObjects::Reserve() method

[Mattia Rizzolo]

https://security-tracker.debian.org/tracker/CVE-2018-5783
https://bugzilla.redhat.com/show_bug.cgi?id=1536179

In PoDoFo 0.9.5, there is an uncontrolled memory allocation in the PoDoFo::PdfVecObjects::Reserve() method (base/PdfVecObjects.h). Remote attackers could leverage this vulnerability to cause a denial of service via a crafted PDF file.

[Dominik Seichter]

I'm wondering what would be a good strategy to address this issue?

PdfVecObjects::Reserve just delegates to std::vector::reserve. It is called from PdfParser with a value directly read from the XRef table. Usually this is followed by the same number of push_back calls on the same vector. So, ideally the call to Reserve is just an optimization (push_back would also reallocate if necessary). I also can see how an attacker could allocate large amounts of memory using a crafted file.

size_t PdfVecObjects::s_maxReserveSize = static_cast<size_t>(64 * 1024); // Never allocate more than 64KB by default 
PdfVecObjects::SetMaxReserveSize(size_t size)
{
    s_maxReserveSize = max;
}

size_t PdfVecObjects::GetMaxReserveSize() 
{
    return s_maxReserveSize;
}

inline void PdfVecObjects::Reserve( size_t size )
{
    if( size <= s_maxReserveSize )
    {
        m_vector.reserve( size );
    } 
    else
    {
        PdfError::DebugMessage( "Call to PdfVecObjects::Reserve with %u is over allowed limit of %u.", size, s_maxReserveSize ); 
    }
}

Does this sound like a reasonable approach to fixing this?
Does anyone have experience with fixes like this? What would be a sensible default value?

[Matthew Brincke]

IMHO yes, but I see two issues:
First, PdfVecObjects::Reserve() is an inline method, so fully updating will require updating its reverse dependencies also (after re-compiling them). To avoid this for the future, couldn't the "inline" specifier be removed (w/o significant hit to performance)?
Second, the format specifiers for size_t quantities are not-so-portable (Windows %Iu with uppercase I, Linux/standard %zu), therefore a new define should be introduced, e.g. PODOFO_SIZE_FORMAT.

[MarkR]

Is an upper limit on the size of PdfVecObjects given by the limit of 8,388,607 indirect objects in a PDF file specified by Table C.1 in Appendix C.2 Architectural limits in PDF 32000-1:2008?

[Matthew Brincke]

This is fixed per my test as of svn r1949, so I close it now.