[poppler] [PATCH] Catalog::getNumPages(): validate page count

[Albert Astals Cid]

El Divendres, 11 de setembre de 2015, a les 08:10:31, Jason Crain va escriure:
> On 2015-09-11 06:59, Even Rouault wrote:
> > Hi,
> > 
> > here's a V2 implementing suggested changes.
> > 
> > Best regards,
> > 
> > Even
> 
>  From 0001-Catalog-getNumPages-validate-page-count.patch:
> > @@ -856,6 +856,18 @@ int Catalog::getNumPages()
> > 
> >        }
> >      
> >      } else {
> >      
> >        numPages = (int)obj.getNum();
> > 
> > +      if (numPages <= 0 ) {
> > +        error(errSyntaxError, -1,
> > +              "Invalid page count {0:d}", numPages);
> > +        numPages = 0;
> > +      }
> > +      else if (numPages > xref->getNumObjects()) {
> > +        error(errSyntaxError, -1,
> > +              "Page count ({0:d}) larger than number of objects
> > ({1:d})",
> > +              numPages, xref->getNumObjects());
> > +        numPages = 0;
> > +      }
> > +
> > 
> >      }
> >      
> >      catDict.free();
> 
> I know I said I liked the idea of comparing the number of pages to the
> number
> of objects, but on second thought, I think I could make a PDF that was
> just
> many copies of the same page so it would be a completely valid PDF that
> has
> more pages than objects.  

Good point, for weird that document may be i agree it's still well formed and 
we should not reject it.

> I don't know of a good way to validate the
> page
> count.  Even going through the page tree might be hard to do right
> without
> leading to an infinite loop, in addition to being slow.

Catalog::cachePageTree goes over the tree, but i agree doing that to calculate 
the num of pages can be meh.

I'd say we just go with using gmalloc_checkoverflow for now?

Cheers,
  Albert

> _______________________________________________
> poppler mailing list
> poppler at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/poppler