[poppler] invalid option can cause SEGV in pdftocairo

[Albert Astals Cid]

El Divendres, 15 de novembre de 2013, a les 22:33:16, suzuki toshiya va 
escriure:
> Hi,

Hi

> 
> During the play with pdftocairo, occasionally I found that
> an invalid option can cause SEGV in pdftocairo;
> 	$ pdftocairo -f 2 -l 1 -svg sample.pdf sample.svg
> 	Segmentation fault.
> 
> The background is simple.
> 
>  989   cairoOut = new CairoOutputDev();
>  990   cairoOut->startDoc(doc);
>  991   if (sz != 0)
>  992     crop_w = crop_h = sz;
>  993   pg_num_len = numberOfCharacters(doc->getNumPages());
>  994   for (pg = firstPage; pg <= lastPage; ++pg) {
> 
> ...
> 
> 1040     if (pg == firstPage)
> 1041       beginDocument(outputFileName, output_w, output_h);
> 1042     beginPage(output_w, output_h);
> 1043     renderPage(doc, cairoOut, pg, pg_w, pg_h, output_w, output_h);
> 1044     endPage(imageFileName);
> 1045   }
> 1046   endDocument();
> 
> As you can see, the document initialization is done in the loop.
> If invalid start/end pages are given (e.g. in upside-down order),
> the beginDocument() is not invoked but endDocument() is invoked.
> 
>  623 static void endDocument()
>  624 {
>  625   cairo_status_t status;
>  626
>  627   if (printing) {
>  628     cairo_surface_finish(surface);
>  629     status = cairo_surface_status(surface);
>  630     if (status)
>  631       error(errInternal, -1, "cairo error: {0:s}\n",
> cairo_status_to_string(status)); 632     cairo_surface_destroy(surface);
>  633     fclose(output_file);
>  634   }
>  635 }
> 
> If endDocument() is invoked without beginDocument(), uninitialized
> surface and output_file are referred, therefore, SEGV is caused.
> 
> The easiest fix would be NULL-initialization (not needed in C++?) of
> surface & output_file, and checking NULL before referring them in
> endDocument(). Patch is attached.

I'd prefer an if check for lastPage < firstPage that tells the user to not 
give the program bad parameters. Can you work on that?

Cheers.
  Albert

> 
> Regards,
> mpsuzuki