Skip to content

XRef::reserve(): fix use-after-free and integer overflow on large XRref /Size

Even Rouault requested to merge rouault/poppler:fix_ossfuzz_62150 into master

This fixes two issues:

  • when using greallocn(), and the reallocation failed, the previous 'entries' array was unexpectedly freed, causing later use-after-free

test.pdf

$ (ulimit -v 1000000; valgrind ./utils/pdftoppm -png test.pdf)
Out of memory
==1251090== Invalid read of size 4
==1251090==    at 0x49F685B: Object::free() (poppler/Object.cc:115)
==1251090==    by 0x4A339BB: ~Object (poppler/Object.h:171)
==1251090==    by 0x4A339BB: XRef::resize(int) (poppler/XRef.cc:459)
==1251090==    by 0x4A32E20: XRef::constructXRef(bool*, bool) (poppler/XRef.cc:877)
==1251090==    by 0x4A32CD4: XRef::XRef(BaseStream*, long long, long long, bool*, bool, std::function<void ()> const&) (poppler/XRef.cc:318)
==1251090==    by 0x4A00531: PDFDoc::setup(std::optional<GooString> const&, std::optional<GooString> const&, std::function<void ()> const&) (poppler/PDFDoc.cc:247)
==1251090==    by 0x4A002DB: PDFDoc::PDFDoc(std::unique_ptr<GooString, std::default_delete<GooString> >&&, std::optional<GooString> const&, std::optional<GooString> const&, void*, std::function<void ()> const&) (poppler/PDFDoc.cc:161)
==1251090==    by 0x49F49EA: LocalPDFDocBuilder::buildPDFDoc(GooString const&, std::optional<GooString> const&, std::optional<GooString> const&, void*) (poppler/LocalPDFDocBuilder.cc:0)
==1251090==    by 0x4A1B1E5: PDFDocFactory::createPDFDoc(GooString const&, std::optional<GooString> const&, std::optional<GooString> const&, void*) (poppler/PDFDocFactory.cc:62)
==1251090==    by 0x4035CA: main (utils/pdftoppm.cc:503)
==1251090==  Address 0x6698648 is 24 bytes inside a block of size 40,960 free'd
==1251090==    at 0x483CA3F: free (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==1251090==    by 0x4927B1C: greallocn(void*, int, int, bool, bool) (goo/gmem.h:0)
==1251090==    by 0x4A33A13: greallocn_checkoverflow (goo/gmem.h:185)
==1251090==    by 0x4A33A13: reserve (poppler/XRef.cc:430)
==1251090==    by 0x4A33A13: XRef::resize(int) (poppler/XRef.cc:446)
==1251090==    by 0x4A32CB5: XRef::XRef(BaseStream*, long long, long long, bool*, bool, std::function<void ()> const&) (poppler/XRef.cc:317)
==1251090==    by 0x4A00531: PDFDoc::setup(std::optional<GooString> const&, std::optional<GooString> const&, std::function<void ()> const&) (poppler/PDFDoc.cc:247)
==1251090==    by 0x4A002DB: PDFDoc::PDFDoc(std::unique_ptr<GooString, std::default_delete<GooString> >&&, std::optional<GooString> const&, std::optional<GooString> const&, void*, std::function<void ()> const&) (poppler/PDFDoc.cc:161)
==1251090==    by 0x49F49EA: LocalPDFDocBuilder::buildPDFDoc(GooString const&, std::optional<GooString> const&, std::optional<GooString> const&, void*) (poppler/LocalPDFDocBuilder.cc:0)
==1251090==    by 0x4A1B1E5: PDFDocFactory::createPDFDoc(GooString const&, std::optional<GooString> const&, std::optional<GooString> const&, void*) (poppler/PDFDocFactory.cc:62)
==1251090==    by 0x4035CA: main (utils/pdftoppm.cc:503)
==1251090==  Block was alloc'd at
==1251090==    at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==1251090==    by 0x4927ACD: grealloc (goo/gmem.h:77)
==1251090==    by 0x4927ACD: greallocn(void*, int, int, bool, bool) (goo/gmem.h:174)
==1251090==    by 0x4A33A13: greallocn_checkoverflow (goo/gmem.h:185)
==1251090==    by 0x4A33A13: reserve (poppler/XRef.cc:430)
==1251090==    by 0x4A33A13: XRef::resize(int) (poppler/XRef.cc:446)
==1251090==    by 0x4A3344E: XRef::constructXRef(bool*, bool) (poppler/XRef.cc:979)
==1251090==    by 0x4A32BF9: XRef::XRef(BaseStream*, long long, long long, bool*, bool, std::function<void ()> const&) (poppler/XRef.cc:304)
==1251090==    by 0x4A00531: PDFDoc::setup(std::optional<GooString> const&, std::optional<GooString> const&, std::function<void ()> const&) (poppler/PDFDoc.cc:247)
==1251090==    by 0x4A002DB: PDFDoc::PDFDoc(std::unique_ptr<GooString, std::default_delete<GooString> >&&, std::optional<GooString> const&, std::optional<GooString> const&, void*, std::function<void ()> const&) (poppler/PDFDoc.cc:161)
==1251090==    by 0x49F49EA: LocalPDFDocBuilder::buildPDFDoc(GooString const&, std::optional<GooString> const&, std::optional<GooString> const&, void*) (poppler/LocalPDFDocBuilder.cc:0)
==1251090==    by 0x4A1B1E5: PDFDocFactory::createPDFDoc(GooString const&, std::optional<GooString> const&, std::optional<GooString> const&, void*) (poppler/PDFDocFactory.cc:62)
==1251090==    by 0x4035CA: main (utils/pdftoppm.cc:503)
  • the logic to exponentially resize the capacity of the array was relying on undefined behaviour of overflow of int. Change that to explictely test the value of the capacity before multiplying by 2.

Merge request reports