Hi zyx,
Thanks for the solid analysis. I agree and reverted the fix I did in 1873.
I am not sure how we PODOFO_RAISE_LOGIC_IF can cause such an error?
It should always be legal to call this:
PODOFO_RAISE_LOGIC_IF( m_stack.empty(), "Can get current graphicsstate!" );
As it translates to:
if( m_stack.empty() ) {
throw ...
}
What for tools do you have in mind? Of course we convert the call to the
macro to using real if-statements here.
Best regards,
Dominik
On Sun, Feb 18, 2018 at 2:37 PM, zyx <z...@gmx.us> wrote:
> On Fri, 2018-01-26 at 17:37 +0100, Dominik Seichter via Podofo-users
> wrote:
> > https://security-tracker.debian.org/tracker/CVE-2017-6845
> > -> I have committed a fix that should address this issue. Maybe
> > someone on the list can review my fix.
>
> Hi,
> I had a look on this, mainly because of the added warnings in compile
> time (the compiler warning is correct, checking for 'NULL == &rhs' is
> wrong, because it means dereferencing such NULL pointer first, which
> can and should lead to a crash on its own) and also in runtime of
> test/unit/podofo-test, and when I run the test case of the CVE, then it
> is not triggered here, the call finishes with the following exception
> instead:
>
> Error: An error 8 ocurred during processing the pdf file
>
> PoDoFo encountered an error. Error: 8 ePdfError_InternalLogic
> Error Description: An internal error occurred.
> Callstack:
> #0 Error Source: .../tools/podofocolor/graphicsstack.cpp:38
> Information: Can push copy on graphicsstack! Stack is empty!
>
> I also added a debug printf() into the PdfColor::operator=() and it is
> never called with a NULL '&rhs' here.
>
> That led me to the place of the exception and it uses
> PODOFO_RAISE_LOGIC_IF(). Looking into its definition, then it throws
> the exception only if the library is compiled with debugging, otherwise
> it does nothing.
>
> I believe the correct change is to revert the revision 1873 and change
> the usage of PODOFO_RAISE_LOGIC_IF() in the public tools. This macro
> should be used only on places where PoDoFo has complete control of, not
> on places which depend on generic/user data. The PDF processing is all
> about generic data, thus even better change might be to enable
> PODOFO_RAISE_LOGIC_IF() always, not only for debug builds.
>
> Am I right? Which of the two options makes more sense from your point
> of view?
>
> Thanks and bye,
> zyx
>
> ------------------------------------------------------------
> ------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> Podofo-users mailing list
> Podofo-users@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/podofo-users
>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Podofo-users mailing list
Podofo-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/podofo-users
