Heap-use-after-free in podofo 0.10.0(main/PdfEncrypt.h:352:47 in PoDoFo::PdfEncrypt::IsMetadataEncrypted()) about podofo HOT 3 CLOSED

Than you for supplying fuzzing tests. Unfortunately I currently don't have much time to look at all of these, so the fix for encryption tests are delayed until I have more time to dig into them. Since there's no warranty, you're also welcome to supply your own patches.

from podofo.

This is looks like an object ownership issue - there are m_Encrypt members in different classes, some of which are copies of each other.

1. PdfParser::ReadObjects creates PdfParser::m_Encrypt
2. PdfParser::ReadObjectsInternal calls PdfParserObject::SetEncrypt with PdfParser::m_Encrypt.get() which sets PdfParserObject::m_Encrypt to use in delay loading
3. PdfMemDocument takes ownership of PdfParser::m_Encrypt via TakeEncrypt (but this doesn't affect PdfParserObject::m_Encrypt)
4. PdfMemDocument::SetEncrypt assigns to m_Encrypt which deletes the unique_ptr originally in PdfParser::m_Encrypt
5. Subsequent delay loads via PdfParserObject reference the now deleted PdfParserObject::m_Encrypt

I think the issue is the m_Encrypt unique_ptrs are actually shared with PdfParserObject

Possible fixes:

1. replace unique_ptr and raw m_Encrypt pointers with std::shared_ptr
2. throw an exception if SetEncryption is called on an encrypted delay loaded document
3. disable delay loading if a document is encrypted

Option 1) seems lowest impact, except it could increase the size of PdfParserObject - and there's one of these for every object in the file. It currently looks like this:

    InputStreamDevice*m_device;
    PdfEncrypt* m_Encrypt;
    bool m_IsTrailer;
   // 3 or 7 bytes of alignment padding inserted by compiler here
    size_t m_Offset;
    bool m_HasStream;
   // 3 or 7 bytes of alignment padding inserted by compiler here
    size_t m_StreamOffset;

At the moment PdfParserObject has 6 bytes (32-bit) or 14 bytes (64-bit) of wasted space due to alignment padding. If the members were re-ordered to eliminate most of the alignment padding and m_Encrypt converted to a shared_ptr the change should be neutral in terms of PdfParserObject size:

    std::shared_ptr<PdfEncrypt> m_Encrypt; // typically 2 * sizeof(void*) in most compiler implementations
    InputStreamDevice*m_device;
    size_t m_Offset;
    size_t m_StreamOffset;
    bool m_IsTrailer;
    bool m_HasStream;
   // 2 bytes or 6 bytes of alignment padding here

32-bit - no change - 6 bytes of alignment padding reduced to 2 bytes and extra pointer in shared_ptr uses 4 bytes
64-bit - no change - 14 bytes of alignment padding reduced to 6 bytes and extra pointer in shared_ptr uses 8 bytes

from podofo.

Hello Mark! I like the suggestion to move to std::shared_ptr. Patch welcome, as I'm overwhelmed by tasks.

from podofo.