Re: [Podofo-users] Next PoDoFo Release 0.9.6

[Mattia Rizzolo]

On Fri, Jan 26, 2018 at 11:35:44PM +0100, Matthew Brincke wrote:
> > Plus this one without CVE that was reported in this ML:  
> > https://blogs.gentoo.org/ago/2017/02/01/podofo-null-pointer-dereference-in-pdfinfoguessformat-pdfinfo-cpp/
> 
> This is *not* fixed yet. I also don't understand why it didn't get
> a CVE entry.

I asked for one back then (it was about the time the workflow to request
CVEs from mitre changed from "random mail on oss-security" to the more
private web form,), and after basically copy-pasting the web page into
the form I got back this message on 2017-02-12:

> [snip]
> You may republish or redistribute this message. We think that someone
> has already posted to oss-security about this issue. To make
> oss-security list members aware that there is no CVE ID assignment,
> you could reply to that oss-security post and include pertinent
> information below.
> [snip]
> As far as we can tell, an end user experiences a loss of functionality
> after the podofopdfinfo command-line tool crashes with a NULL pointer
> dereference (because the end user can completely work around this by
> not repeating the specific command-line invocation, there would be no
> security impact).
> 
> Although some parts of PoDoFo are library code that could be reached
> from an arbitrary application, the reported code in
> PdfInfo::GuessFormat appears to be reachable only from the
> podofopdfinfo command-line tool.
> 
> Thus, we are not assigning a CVE ID unless there is additional
> information about a security impact.
> 
> - --
> CVE Assignment Team


After all I didn't redistributed the message for some reason (probably
I was just too lazy).
So it seems the reason the CVE was rejected is only because the crash
doesn't happen in the library, but in the tool itself.

-- 
regards,
Mattia Rizzolo

GPG Key: 66AE 2B4A FCCF 3F52 DA18  4D18 4B04 3FCD B944 4540  .''`.
more about me:  https://mapreri.org : :'  :
Launchpad user: https://launchpad.net/~mapreri  `. `'`
Debian QA page: https://qa.debian.org/developer.php?login=mattia  `-


signature.asc
Description: PGP signature
--
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

Re: [Podofo-users] Next PoDoFo Release 0.9.6

[Matthew Brincke]

[ Left Dominik in To to help him follow this thread, fixed text typos ]

Hello Dominik, hello all,

> Dominik Seichter via Podofo-users has written on 26 January 2018 at 17:37: 
>  
> 
> Hi Mattia,
>  
> Thanks for the good summary! Let me comment on the open issues.
>  
> Unfixed security issues: 
... snip ...
> 
> https://security-tracker.debian.org/tracker/CVE-2017-8053
> -> Please see proposed patch in attachment. Can somebody test/review?
> 

In line 13 of the patch, there are typos, it should be "already visited",
line 14 doesn't really fit (which object?), and in general, shouldn't
there be a maximum recursion depth which is checked for, to prevent a
stack overflow? AFAICS there is no standard function/method to check
available stack space ;-( ...

> https://security-tracker.debian.org/tracker/CVE-2017-8054
> -> This was fixed by zyx in revision: 1872. I have a test PDF
>for this and cannot reproduce this issue anymore.

The fix was provided by Matthias Brinke 
(stands for "PoDoFo security contributor", I'm a friend of his) on the
Debian Bug Tracking System: https://bugs.debian.org/860995

>  
> Plus this one without CVE that was reported in this ML:  
> https://blogs.gentoo.org/ago/2017/02/01/podofo-null-pointer-dereference-in-pdfinfoguessformat-pdfinfo-cpp/

This is *not* fixed yet. I also don't understand why it didn't get
a CVE entry.

> (CVE-2017-8054 had a tentative patch)
> -> Seems same as above and seems fixed.

The CVE, yes, contrary to the other one without a CVE entry.
 
>  
> A threading problem: 
>  https://sourceforge.net/p/podofo/mailman/message/35915862/
> -> There is no need to make the matrix for XObjects static, so I made
>it a normal member. Same for s_procset in PdfCanvas. So should be
>fixed with my last commit.

As you said in your next e-mail to the ML the double-checked locking pattern
isn't fixed yet: https://sourceforge.net/p/podofo/mailman/message/36205920/

>  
> A copyright issue: 
>  https://sourceforge.net/p/podofo/mailman/message/35633858/
> -> We still do not have a fix for this.
>

I recommend libunistring2 to fix it, but haven't used it yet.
  
> Regarding bug tracker: Yes, a bug tracker would be nice. But I can barely
> follow the mailing list, so I do not feel I able to setup and maintain a
> bug tracker. If somebody volunteers, I would not object. 
> BTW: Just found this on Sourceforge: 
> https://sourceforge.net/p/podofo/bugs/?source=navbar 
> Anybody has experience with this? Shall we just use this feature?
> 

Peter Linnell has said something like that, yes (2.5 months ago on this ML):
https://sourceforge.net/p/podofo/mailman/message/36112914/

> 
> Best regards,
>  Dominik
> 

Best regards, mabri
 
> On Mon, Jan 22, 2018 at 7:25 PM, Mattia Rizzolo  wrote: 
> 
> > [ explicitly put Dominik in To, as I'm unsure how much he follows the 
> >  ML himself… ] 
> >  
> >  On Sun, Jan 14, 2018 at 08:48:05PM +0100, Dominik Seichter via 
> > Podofo-users wrote:
> > > The last version of PoDoFo was released almost a year ago on February 2nd
> > > 2017. I have seen many patches on the mailing list and also many commits 
> > > to
> > > SVN over the last year. So, I think it is time for a new PoDoFo release
> > > 0.9.6.
> > >
> > > As there might have been patches, which either Zyx or I have missing, I
> > > would suggest the following release time line.
> >  
> >  In December there was a similar email to this going on, asking about a 
> >  new release.  It was pointed out that there are still known unfixed CVEs 
> >  and other important issues. 
> >  See https://sourceforge.net/p/podofo/mailman/message/36151169/
> >  
... snip ...
> > 
> >  Who knows what more… 
> >  While you are here, would you reconsider opening a bug tracker 
> >  somewhere?  When it was proposed in the past in this ML, nobody was 
> >  against it, but everybody deferred to you iirc. 
> >  
> >  --
> >  regards,
> >                          Mattia Rizzolo
> >  
> >  GPG Key: 66AE 2B4A FCCF 3F52 DA18  4D18 4B04 3FCD B944 4540      .''`.
> >  more about me:  https://mapreri.org                             : :'  :
> >  Launchpad user: https://launchpad.net/~mapreri                  `. `'`
> >  Debian QA page: https://qa.debian.org/developer.php?login=mattia  `-
> >

--
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

Re: [Podofo-users] Patch replaced: Wannabe-namespaced internal function converted to private method

[Matthew Brincke]

Hello zyx, hello all,

> zyx  has written on 14 January 2018 at 11:40:
> 
> 
> On Thu, 2018-01-04 at 00:59 +0100, Matthew Brincke wrote:
> > > the internal function name PODOFO_Base14FontDef_FindBuiltinData
> > > looks like it should be in a namespace and a class IMHO, therefore
> > > I've converted it to a private method with access only from
> > > PdfFontFactory.
> 
>   Hi,
> I'm sorry, but what is the idea behind the change, please? If only the
> above, then, well, from my point of view, it doesn't make much sense. I
> see that it makes many things more complicated, defines classes inside

no, that was only my original idea, before I knew how to do it in C++.
The reasons why I've made it so "complicated" are that IMHO it's important
to isolate an internal "function" (static method, so "private" applies) as
much as possible (a tenet of OOP AFAIK) and the C++ syntax rules. I've left
off PODOFO_LOCAL only because that's (probably by far) not not the only
location to insert it and that such a change would be advisable later anyway
(by Mattia Rizzolo [1]).

> classes (while possible, I do not like it myself), requirement of
> 'friend' classes, which also kind of points into a bad/suboptimal API

Without the "friend" declarations I'd have a design with much less isolation:
One "friend function" declaration was there before, but C++ forbids them for
private methods, so I would have had to either make the method public (not
acceptable for me because:
- that would've added to the API whereas I'd like to keep it internal and
- I had hoped that it would be easier to get a not-to-API change committed
- (PODOFO_Base14FontDef_FindBuiltinData isn't in the PoDoFo API, I hope?),
- I have doubts it should be public because of OOP and its implementation
- because of these reasons I couldn't get rid of that "friend" declaration)

or change it to declare the whole class PdfFontFactory a "friend" which
would be worse because AFAIK the best design with a "friend" is (the) one
which makes the breach to isolation minimal, that is, (in this case) which
gives access to the minimum number of methods (one in this case). I didn't
want to have the new internal class needed for this to be at namespace level
because I think that is for public API classes only.
The "friend PdfFontFactory" declaration in there (nested class Builtin) is
necessary because otherwise the FindMetrics() method would need to be public
(C++ gives free access from nested to outer class, not the other way around).

The other nested class is required to avoid giving the (public API?) class
PdfFontCache access to the internal Base14 font metrics ("needs to know"
principle). The "friend" declaration there is give access only to the class
which needs it (PdfFontCache, restricting it to method level would've been
more complicated still, so much that I ruled it inelegant and avoided it).

> design, and also:
> 
> > > Should someone desire access to any of this info outside PoDoFo,
> > > some workaround is possible: e.g. create the font, query its
> > > metrics and check if they can be converted (dynamic_cast) to
> > > PdfFontMetricsBase14.
> 
> which is quite discouraging for me.

Why is it discouraging for you, please?
I only think of it as an interim workaround before API will be introduced
to handle Base14 font metrics better, that is, without giving access (to
manipulate, even, with PdfFont::GetFontMetrics2(), at least a copy)
to the internal values. At that time, it should be deprecated IMHO. 

> 
> Just my opinion. If the maintainer will wish to commit the change, then
> I'm not against it, I'm just not in favor of it myself.

I'd like to discuss a roadmap to API changes with Dominik, project leader,
anyway, do you mean him? I thank you in advance for a not-put-off answer.

>   Bye,
>   zyx
> 

Best regards, mabri

[1] https://sourceforge.net/p/podofo/mailman/message/36112580/ (last sentence)

--
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

Re: [Podofo-users] Thread safety

[Dominik Seichter via Podofo-users]

Hi Michall,

Sorry for the terribly late reply to your email.
I replaced the static arrays with local variables which is totally fine for
these case: https://sourceforge.net/p/podofo/code/1875/

The invalid double checked locking for singletons needs a separate fix
though.

Best regards,
 Dominik

On Tue, Jun 27, 2017 at 10:17 PM, Michal Sudolsky 
wrote:

> Hi,
>
> There are many initialised static local variables which is thread-safe
> only in C++11 and above what is ok.
>
> What is not ok is that there are two class member static variables which
> are initialised lazily in some member function.
> These cases are definitely not thread-safe. There should be mutex at least
> or they could be simply declared non-static.
>
> PdfCanvas:
>
>  private:
>> /** The procset is the same for all
>>  *  PdfContents objects
>>  */
>> *static **PdfArray s_procset;*
>
>
>
>
>>  const PdfArray & PdfCanvas::GetProcSet()
>> {
>> if( s_procset.empty() )
>> {
>> s_procset.push_back( PdfName( "PDF" ) );
>> s_procset.push_back( PdfName( "Text" ) );
>> s_procset.push_back( PdfName( "ImageB" ) );
>> s_procset.push_back( PdfName( "ImageC" ) );
>> s_procset.push_back( PdfName( "ImageI" ) );
>> }
>> return s_procset;
>> }
>
>
> PdfXObject:
>
>  private:
>> *static **PdfArray  s_matrix;*
>
>
> void PdfXObject::InitXObject( const PdfRect & rRect, const char*
>> pszPrefix )
>> {
>> PdfVariantvar;
>> ostringstream out;
>> PdfLocaleImbue(out);
>
>
>
> // Initialize static data
>> if( s_matrix.empty() )
>> {
>> // This matrix is the same for all PdfXObjects so cache it
>> s_matrix.push_back( PdfVariant( static_cast(PODOFO_
>> LL_LITERAL(1)) ) );
>> s_matrix.push_back( PdfVariant( static_cast(PODOFO_
>> LL_LITERAL(0)) ) );
>> s_matrix.push_back( PdfVariant( static_cast(PODOFO_
>> LL_LITERAL(0)) ) );
>> s_matrix.push_back( PdfVariant( static_cast(PODOFO_
>> LL_LITERAL(1)) ) );
>> s_matrix.push_back( PdfVariant( static_cast(PODOFO_
>> LL_LITERAL(0)) ) );
>> s_matrix.push_back( PdfVariant( static_cast(PODOFO_
>> LL_LITERAL(0)) ) );
>> }
>
>
>
> This DLCP singleton is also not thread-safe on weakly ordered processors:
>
> const PdfEncoding* PdfEncodingFactory::GlobalPdfDocEncodingInstance()
>> {
>> if(!s_pDocEncoding) // First check
>> {
>> Util::PdfMutexWrapper wrapper( PdfEncodingFactory::s_mutex );
>>
>> if(!s_pDocEncoding) // Double check
>> s_pDocEncoding = new PdfDocEncoding();
>> }
>> return s_pDocEncoding;
>> }
>
> Can be fixed in C++11 http://preshing.com/20130930/double-checked-locking-
> is-fixed-in-cpp11/
>
>
>
> 
> --
> 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