+ if ( s_nRecursionDepth > s_maxRecursionDepth )
+ {
+ // avoid stack overflow on documents that have circular cross references,
loops
+ // or very deeply nested structures, can happen with
+ // /Prev entries in trailer and XRef streams (possible via a chain of
entries with a loop)
+ // /Kids entries that loop back to self or parent
+ // deeply nested Dictionary or Array objects (possible with lots of
[[[[[[[[]]]]]]]] brackets)
+ // mutually recursive loops involving several objects are possible
+ PODOFO_RAISE_ERROR( ePdfError_InvalidXRef );
+ }Not all these cases are invalid xref errors. On Wed, Feb 2, 2022 at 7:16 PM Mark Rogers <mark.rog...@powermapper.com> wrote: > Hi Everyone > > > > Here are patches for recursive stack consumption, which should fix > CVE-2018-8002, CVE-2021-30470, CVE-2021-30471, CVE-2020-18971 > > > > This works by refactoring the recursion guard and making it a nested class > of PdfTokenizer (as it’s mostly used by the tokenizer and parser). As > agreed earlier in this thread the patch means that PoDoFo requires C++ 11 > if compiled with PODOFO_MULTI_THREAD > > > > The patch has been tested against the CVE PoC files, and the new unit > tests. It’s also been tested in production for 2 months on macOS (64-bit) > and Windows (32-bit) > > > > We haven’t tested on Linux. This might be relevant for the > ParserTest::getStackOverflowDepth() unit test method which calculates an > overflow depth for each platform that causes stack overflow without > exhausting the heap (although the calculation should be the same as macOS > since they both use the same System V AMD64 ABI). > > > > Best Regards > > Mark > > > > Mark Rogers - mark.rog...@powermapper.com > > PowerMapper Software Ltd - www.powermapper.com > > Registered in Scotland No 362274 Quartermile 2 Edinburgh EH3 9GL > > > > > > > > > > > > *From: *Michal Sudolsky <sudols...@gmail.com> > *Date: *Thursday, 25 November 2021 at 18:25 > *To: *Christopher Creutzig <ccreu...@mathworks.com> > *Cc: *"podofo-users@lists.sourceforge.net" < > podofo-users@lists.sourceforge.net> > *Subject: *Re: [Podofo-users] PoDoFo and recursive stack consumption CVEs > > > > > > > > On Thu, Nov 25, 2021 at 4:58 PM Christopher Creutzig < > ccreu...@mathworks.com> wrote: > > > Ok, I can submit a patch which uses C++11 thread_local when > > PODOFO_MULTI_THREAD > is defined. The recursion guard definition will look like this: > > > > > > #if defined(PODOFO_MULTI_THREAD) > > > static int thread_local s_nRecursionDepth; // PoDoFo with threading > support requires C++11 compiler with thread_local > > > #else > > > static int s_nRecursionDepth; // PoDoFo is single threaded > > > #endif > > > > > > Does that work for everyone? > > > > Looks good to me, and the comment is hopefully explanation enough if > anyone runs into a compile time error. Please do include a doc patch > stating the requirement. > > > > Can we get a macro that creates this thread-local integer and the > recursion guard object all in one go, with the connotation that the > recursion guard is meant to usually be applied to each affected function > entry point separately? (Unless that is not what it is meant to do. I think > we could just as well make an argument for a single recursion depth counter > per thread, which then probably should become a static member of the > recursion guard class.) > > > > I think that it was meant that there will be just a single recursion > counter per thread. > > > > > > > > Cheers, > > Christopher > > > > The MathWorks GmbH | Friedlandstr.18 | 52064 Aachen | District Court > Aachen | HRB 8082 | Managing Directors: Bertrand Dissler, Steven D. Barbo, > Jeanne O’Keefe > > > > > > > > *From:* Mark Rogers <mark.rog...@powermapper.com> > *Sent:* Thursday, November 25, 2021 16:33 > *To:* Christopher Creutzig <ccreu...@mathworks.com>; Michal Sudolsky < > sudols...@gmail.com> > *Cc:* podofo-users@lists.sourceforge.net > *Subject:* Re: [Podofo-users] PoDoFo and recursive stack consumption CVEs > > > > >>> I like this idea. As PODOFO_MULTI_THREAD will enable C++11 > > >>Is there a chance we might get there? Who would be able to make that > decision? > > >consider the decision being made. Again, from my point of view. In > > >other words, feel free to provide a patch with the suggested changes. > > > > Ok, I can submit a patch which uses C++11 thread_local when > PODOFO_MULTI_THREAD > is defined. The recursion guard definition will look like this: > > > > #if defined(PODOFO_MULTI_THREAD) > > static int thread_local s_nRecursionDepth; // PoDoFo with threading > support requires C++11 compiler with thread_local > > #else > > static int s_nRecursionDepth; // PoDoFo is single threaded > > #endif > > > > Does that work for everyone? > > > > > Not when the user of podofo already used some 64 KB before calling > podofo. To me it seems more reasonable to use a more conservative value > which would not consume more than some half (or tenth?) of the available > stack in the worst case. > > > > I’ll also reduce the 500 max recursion depth as suggested (probably to 256) > > > > And I’ll also include the new parser unit tests which test for deep > recursion and reference loops > > > > We’re also testing a patch for CVE-2018-20797. This is caused by an > invalid negative value for one of the FlateDecode compression parameters > which results in a call to podofo_calloc( -14 ) == podofo_calloc( > 0xfffffffffffffff2 ) > > > > Best Regards > > Mark > > > > Mark Rogers - mark.rog...@powermapper.com > > PowerMapper Software Ltd - www.powermapper.com > > Registered in Scotland No 362274 Quartermile 2 Edinburgh EH3 9GL > > > > > > > > *From: *Christopher Creutzig <ccreu...@mathworks.com> > *Date: *Thursday, 25 November 2021 at 07:16 > *To: *Michal Sudolsky <sudols...@gmail.com> > *Cc: *"PowerMapper.com" <mark.rog...@powermapper.com>, " > podofo-users@lists.sourceforge.net" <podofo-users@lists.sourceforge.net> > *Subject: *RE: [Podofo-users] PoDoFo and recursive stack consumption CVEs > > > > >> If we want to avoid UB in the multithreaded world, I’m afraid we will > have to make a C++11 compiler a requirement, as C++03 never acknowledged > the existence of threads. (That is not limited to this place, a lot of > methods like PdfEncodingFactory::GlobalPdfRomanEncodingInstance are not > currently threadsafe in C++03, as discussed earlier.) > > > That is not thread-safe even in C++11. > > > > True, but C++11 or later would give us the tools to make it thread-safe. > > > > > Except that some things are not so available as threads like for > example thread_local and atomic operations. > > > > thread_local equivalents are available for g++, clang, and MSVC. That > covers the compilers listed in > https://github.com/NickNaso/PoDoFo/blob/master/README.md#installation_with_cmake. > See my proposed PODOFO_THREAD_LOCAL in > https://sourceforge.net/p/podofo/mailman/message/37389082/. > > > > > I like this idea. As PODOFO_MULTI_THREAD will enable C++11 > > > > Is there a chance we might get there? Who would be able to make that > decision? > > > > > > Cheers, > > Christopher > > > > The MathWorks GmbH | Friedlandstr.18 | 52064 Aachen | District Court > Aachen | HRB 8082 | Managing Directors: Bertrand Dissler, Steven D. Barbo, > Jeanne O’Keefe > > > > _______________________________________________ > Podofo-users mailing list > Podofo-users@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/podofo-users > >
_______________________________________________ Podofo-users mailing list Podofo-users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/podofo-users
