Bug 697286

Summary: ps2pdf: SIGSEGV in dict_next()
Product: Ghostscript Reporter: David Kaspar // Dee'Kej <deekej>
Component: PDF WriterAssignee: Ken Sharp <ken.sharp>
Status: RESOLVED FIXED    
Severity: normal CC: chris.liddell
Priority: P4    
Version: master   
Hardware: PC   
OS: Linux   
Customer: Word Size: ---
Attachments: Proposed workaround
core-dump-9.07
.ps causing SIGSEGV
coredump for 9.20 (15cf5b905)

Description David Kaspar // Dee'Kej 2016-11-02 04:44:50 UTC
Created attachment 13068 [details]
Proposed workaround

Hello,

we received an information that ghostscript will segfault in a specific case. I can't provide the *.ps causing this segfault yet, I'm still waiting for customer approval of sharing it.

So far, I'm providing the core dump and a proposed patch from one of my colleagues.

The coredump comes from version 9.07, but I have tested this on latest master vanilla build (from git repository), and the segfault occurs there as well, in the same place. Basic backtrace from 9.21:
------------------------------------------
Program received signal SIGSEGV, Segmentation fault.
0x000000000083e308 in dict_next ()
(gdb) bt
#0  0x000000000083e308 in dict_next ()
#1  0x0000000000820949 in cid_font_data_param ()
#2  0x0000000000821d70 in zbuildfont11 ()
#3  0x00000000008424e3 in interp ()
#4  0x0000000000842df5 in gs_interpret ()
#5  0x00000000008379df in gs_main_run_string_with_length ()
#6  0x0000000000839154 in run_string ()
#7  0x00000000008392c8 in runarg ()
#8  0x000000000083adf8 in gs_main_init_with_args ()
#9  0x00000000004643f2 in main ()

The call I used to reproduce this issue (for latest vanilla build):
------------------------------------------------------------------
> gs -sDEVICE=pdfwrite -sstdout=%stderr -o out.pdf -f mfg.ps

If you remove the '-sstdout=%stderr', it will convert the *.ps file correctly.

> gs -sDEVICE=pdfwrite -o out.pdf -f mfg.ps
... will segfault for git branch 'gs920'.


According to my colleague (please ignore the lines' offset):
============================================================
In dict_next(), invalid address was set into vp, and segfault happened the following line 871.

    860 dict_next(const ref * pdref, int index, ref * eltp /* ref eltp[2] */ )
    862     dict *pdict = pdref->value.pdict;
    863     ref *vp = pdict->values.value.refs + index;
... snip ...
    871             eltp[1] = *vp;

The value of pdict->values.value.refs was 0 here unexpectedly.

(gdb) print pdict->values.value.refs
$2 = (ref *) 0x0


So, it seems 'pdref' (1st arg) doesn't have a valid dictionary data.

     42 cid_font_data_param(os_ptr op, gs_font_cid_data *pdata, ref *pGlyphDirectory)
... snip ...
     60     if (dict_find_string(op, "GlyphDirectory", &pgdir) <= 0) {
... snip ...
     66     if (r_has_type(pgdir, t_dictionary) || r_is_array(pgdir)) {
... snip ...
     79         index = dict_first(pgdir);
     80         while (index >= 0) {
     81             index = dict_next(pgdir, index, (ref *)&element);

'pgdir' is set in line 60 of cid_font_data_param(). 

The input ps file has a line including "GlyphDirectory"
It's possible that it could not get the correct data from the ps file.
But, I created a (workaround) patch to check the value of pgdir->value.pdict->values.value.refs. And, it didn't fail with segfault, and the pdf file was created.

=========================

Once I get the approval from our customer, I will attach the *.ps file causing this segfault as well.
Comment 1 David Kaspar // Dee'Kej 2016-11-02 04:46:07 UTC
Created attachment 13069 [details]
core-dump-9.07

Core dump from ghostscript-9.07.
Comment 2 Ken Sharp 2016-11-02 06:16:19 UTC
(In reply to David Kaspar [Dee'Kej] from comment #0)

> we received an information that ghostscript will segfault in a specific
> case. I can't provide the *.ps causing this segfault yet, I'm still waiting
> for customer approval of sharing it.

We're going to need the input file before we can possibly investigate. I don't think checking for an explicit NULL is going to be sufficient in general. We really need to understand why the refs parameter is NULL at this point. Otherwise we run the risk of simply masking the problem, only for it to re-emerge under slightly different conditions.

Since you say this occurs with master can you please also provide the SHA of the Git commit you have used and the word length of the build (32 or 64 bit). I'm assuming that by 'vanilla build' you mean you haven't applied any patches, aren't building with system shared libraries and haven't added any extra compiler flags.

> ------------------------------------------
> Program received signal SIGSEGV, Segmentation fault.
> 0x000000000083e308 in dict_next ()
> (gdb) bt
> #0  0x000000000083e308 in dict_next ()
> #1  0x0000000000820949 in cid_font_data_param ()
> #2  0x0000000000821d70 in zbuildfont11 ()
> #3  0x00000000008424e3 in interp ()
> #4  0x0000000000842df5 in gs_interpret ()
> #5  0x00000000008379df in gs_main_run_string_with_length ()
> #6  0x0000000000839154 in run_string ()
> #7  0x00000000008392c8 in runarg ()
> #8  0x000000000083adf8 in gs_main_init_with_args ()
> #9  0x00000000004643f2 in main ()

I notice that the back trace does not  include any code from the pdfwrite device, this is in the PostScript interpreter. I suspect this is *NOT* a pdfwrite problem, but a general font problem, its likely that I'll reassign this to a more appropriate engineer after some testing.


> The call I used to reproduce this issue (for latest vanilla build):
> ------------------------------------------------------------------
> > gs -sDEVICE=pdfwrite -sstdout=%stderr -o out.pdf -f mfg.ps
> 
> If you remove the '-sstdout=%stderr', it will convert the *.ps file
> correctly.

> > gs -sDEVICE=pdfwrite -o out.pdf -f mfg.ps
> ... will segfault for git branch 'gs920'.


Which means this may be very hard to reproduce, since that 'should' certainly have no effect on the action of dictionaries, and therefore sounds like its a memory error/corruption related problem.


> The input ps file has a line including "GlyphDirectory"
> It's possible that it could not get the correct data from the ps file.

Without seeing the file I cannot be certain, but this would be possible. In which case a PostScript error should have been thrown at a higher level. Testing against NULL doesn't 'look' like the correct approach, but without seeing the input we cannot tell.
Comment 3 David Kaspar // Dee'Kej 2016-11-02 08:44:42 UTC
(In reply to Ken Sharp from comment #2)
> We're going to need the input file before we can possibly investigate. I
> don't think checking for an explicit NULL is going to be sufficient in
> general. We really need to understand why the refs parameter is NULL at this
> point. Otherwise we run the risk of simply masking the problem, only for it
> to re-emerge under slightly different conditions.
Yes, I completely understand. I have informed customer about the situation and that you need the file for it. I'm still waiting for they to reply.

> Since you say this occurs with master can you please also provide the SHA of
> the Git commit you have used and the word length of the build (32 or 64
> bit). I'm assuming that by 'vanilla build' you mean you haven't applied any
> patches, aren't building with system shared libraries and haven't added any
> extra compiler flags.
Yes, it was compiled with:
------------
./autogen.sh
make
make check
sudo make install

... No shared libraries and no patches were used.

Here are the SHA hashes of commits and the commands used to reproduce this:
-----------
commit 8dcec8cc076a0cf8350ca7a6ec1d3136812e2a24
Author: Robin Watts <robin.watts@artifex.com>
Date:   Tue Nov 1 18:40:04 2016 +0000

    Bug 697186: Workaround JPEG lib bug.
    
    Commit fix for overflow. Awaiting response from IJG.

commit 15cf5b90522f902b554c4ee44d5e424cb451f7b1
Author: Chris Liddell <chris.liddell@artifex.com>
Date:   Tue Sep 20 20:33:21 2016 +0100

    Changelog update for 9.20 RC2

For both of the commits above, I was able to reproduce this when running this command:
> gs -dSAFER -sDEVICE=pdfwrite -o out.pdf -f mfg.ps

UPDATE: It seems that sometimes the conversion succeeds. Right now it seems that adding -dSAFER will cause the conversion to segfault.

> I notice that the back trace does not  include any code from the pdfwrite
> device, this is in the PostScript interpreter. I suspect this is *NOT* a
> pdfwrite problem, but a general font problem, its likely that I'll reassign
> this to a more appropriate engineer after some testing.
I see. I wasn't sure about which sub-component to assign it. I will leave this up to your judgement. :)
Comment 4 David Kaspar // Dee'Kej 2016-11-02 08:45:49 UTC
One more note: It has been tested in 64-bit environment (Fedora 24 in VM).
Comment 5 David Kaspar // Dee'Kej 2016-11-03 03:57:12 UTC
Created attachment 13078 [details]
.ps causing SIGSEGV

Unfortunately, customer requested to not share the original *.ps file, because it contained sensitive information for them.

However, they have provided us with a non-sensitive *.ps file, which is causing the segfault as well. I have checked the backtrace, which is little bit different, but ultimately the program receives SIGSEGV at the same line [in function -- dict_next()].
Comment 6 Ken Sharp 2016-11-03 04:06:38 UTC
(In reply to David Kaspar [Dee'Kej] from comment #5)

> Unfortunately, customer requested to not share the original *.ps file,
> because it contained sensitive information for them.
> 
> However, they have provided us with a non-sensitive *.ps file, which is
> causing the segfault as well. I have checked the backtrace, which is little
> bit different, but ultimately the program receives SIGSEGV at the same line
> [in function -- dict_next()].

Well, obviously we cannot be certain that its the same problem, but the probability looks high. Presumably the original reporter is willing to retest the original file after a solution is available.
Comment 7 David Kaspar // Dee'Kej 2016-11-03 04:16:16 UTC
Created attachment 13079 [details]
coredump for 9.20 (15cf5b905)

This is the coredump from ghostcript-9.20 vanilla build on Fedora 24 (64 bit) [commit 15cf5b90522f902b554c4ee44d5e424cb451f7b1]. The broken.ps caused the SIGSEGV in this coredump.
Comment 8 David Kaspar // Dee'Kej 2016-11-03 04:18:42 UTC
(In reply to Ken Sharp from comment #6)
> Well, obviously we cannot be certain that its the same problem, but the
> probability looks high. Presumably the original reporter is willing to
> retest the original file after a solution is available.

I understand your position. However, I cannot force the customer into sharing sensitive data. :-/

Anyway, they seem to be very communicative, it has high priority for them, so I bet they will gladly test the solution in their environment as well.
Comment 9 Chris Liddell (chrisl) 2016-11-03 07:54:54 UTC
Fixed in:

http://git.ghostscript.com/?p=user/chrisl/ghostpdl.git;a=commitdiff;h=a73e3cf1