The 500 limit should be enough for a 32-bit release build with a 256 KB stack
(it would use about 195 KB)
I think there should be a user configurable limit (PdfRecursionGuard::
SetMaxRecursionDepth) to override the 500 limit on systems with unusually small
stacks - some embedded systems have less than 10KB of stack.
Also worth discussing – should it be possible to disable the recursion guard
completely with SetMaxRecursionDepth(0) ? This is a bad idea with untrusted
input, but might make sense in some situations.
We’ve written a lot of new parser unit tests to test deeply nested and looping
PDF structures. We’ll submit these along with a patch - these tests make it
easy to experiment with different patches for the same issue.
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: Saturday, 20 November 2021 at 22:08
To: "PowerMapper.com" <mark.rog...@powermapper.com>
Cc: Christopher Creutzig <ccreu...@mathworks.com>,
"podofo-users@lists.sourceforge.net" <podofo-users@lists.sourceforge.net>
Subject: Re: [Podofo-users] PoDoFo and recursive stack consumption CVEs
Is option 3) worth investigating?
It seems like the best solution in current circumstances. Another solution
could be to eliminate all recursion but thread_local recursion counter is
simpler to do.
#else
// fallback to process global recursion count – overcounts depth if PDFs
processed in parallel on multiple threads, same result as thread_local if
process single threaded
#define PODOFO_THREAD_LOCAL
#endif
But this fallback would cause UB unless all access to t_nRecursionDepth is
atomic or guarded by mutex.
In a release build on x86/x64 each recursive ReadArray call loop uses about 400
bytes of stack.
* Windows IIS 32-bit worker processes – 256 KB max stack (stack overflows
with 655 ‘[‘ characters)
So would the limit of 500 as default be enough? It should be far enough from
value which would cause stack overflow.
I had a look at the patch in https://sourceforge.net/p/podofo/tickets/25/#51b9.
That’s a simpler solution than the changes I proposed for PdfRecursionGuard.
That patch introduces UB (same reason as above).
_______________________________________________
Podofo-users mailing list
Podofo-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/podofo-users
