Bug 700141

Summary: Type confusion in setpattern
Product: Ghostscript Reporter: Man Yue Mo <mmo>
Component: PS InterpreterAssignee: Ken Sharp <ken.sharp>
Status: RESOLVED FIXED    
Severity: normal CC: cbuissar, omarandemad
Priority: P4    
Version: unspecified   
Hardware: PC   
OS: Linux   
Customer: Word Size: ---

Description Man Yue Mo 2018-11-08 11:35:29 UTC
Hi,

There is a missing type check in line 292 of zcolor.c:

http://git.ghostscript.com/?p=ghostpdl.git;a=blob;f=psi/zcolor.c;h=74b428801eda5c75d70cf55e88c407484b554527;hb=5a4fec2a34af925993192e197ab666fe542b79d3#l292

Here `pPatInst` comes from the first array element of `pImpl`

http://git.ghostscript.com/?p=ghostpdl.git;a=blob;f=psi/zcolor.c;h=74b428801eda5c75d70cf55e88c407484b554527;hb=5a4fec2a34af925993192e197ab666fe542b79d3#l289

which comes from `op`:

http://git.ghostscript.com/?p=ghostpdl.git;a=blob;f=psi/zcolor.c;h=74b428801eda5c75d70cf55e88c407484b554527;hb=5a4fec2a34af925993192e197ab666fe542b79d3#l286

The type of `pPatInst` is not checked and is used in `r_ptr`, which accesses its `pstruct` value and then cast it into `gs_pattern_instance_t`. As `op` is an untrusted argument, this can lead to type confusion issue when parsing malicious postscript. (Access to arbitrary pointer)

For example:

./gs -q -sDEVICE=ppmraw -dSAFER
GS><< /Implementation [16#4141414141] >> setpattern
Segmentation fault (core dumped)

This is tested with a build of commit `5a4fec2` (latest of current master)

Thank you very much for your help and please let me know if there is anything I can help or if you have trouble reproducing the issue. Thanks.

Best Regards,

Man Yue Mo
Comment 1 Ken Sharp 2018-11-08 15:26:49 UTC
Fixed in commit http://git.ghostscript.com/?p=ghostpdl.git;a=commit;h=693baf02152119af6e6afd30bb8ec76d14f84bbf

Thanks for the report!
Comment 2 Man Yue Mo 2018-11-08 16:58:28 UTC
Thanks for the quick response! I can confirm that the fix has prevented the issue. Would you be requesting a CVE for this or would you like me to do so? Thanks.
Comment 3 Ken Sharp 2018-11-08 17:13:25 UTC
(In reply to Man Yue Mo from comment #2)
> Thanks for the quick response! I can confirm that the fix has prevented the
> issue. Would you be requesting a CVE for this or would you like me to do so?
> Thanks.

I'm not sure what our status is with CVE's. If you want one, probably best to go ahead and do it.
Comment 4 Man Yue Mo 2018-11-09 09:35:00 UTC
Thanks. I've requested a CVE ID. I'll let you know when I got the ID. Are you happy with the following description?

The ghostscript setpattern operator did not properly validate certain types. A specially crafted PostScript document could exploit this to crash ghostscript or, possibly, execute arbitrary code in the context of the ghostscript process.

Thanks.
Comment 5 Ken Sharp 2018-11-09 09:40:59 UTC
(In reply to Man Yue Mo from comment #4)

> The ghostscript setpattern operator did not properly validate certain types.
> A specially crafted PostScript document could exploit this to crash
> ghostscript or, possibly, execute arbitrary code in the context of the
> ghostscript process.
> 
> Thanks.

I'm doubtful you could execute arbitrary code, though I'm no expert in the field. I've no problems with the description.
Comment 6 Ken Sharp 2018-11-09 09:42:28 UTC
Forgot to mention it, not sure if its relevant, the fix is in the Release Candidate for 9.26 which is available as of about 30 minutes ago. Obviously it'll be in the 9.26 release when that rolls out.
Comment 7 Man Yue Mo 2018-11-09 10:04:42 UTC
Thanks. The 'execute arbitrary code' clause is rather standard for this kind of issues (e.g. https://access.redhat.com/security/cve/cve-2018-15909) It simply states the maximum possible impact. I personally agree with you that it probably can't execute arbitrary code (and I haven't tried it myself either) on its own and do not intend to exaggerate the issue, but am just following standard practices. Thanks!
Comment 8 Man Yue Mo 2018-11-09 11:39:38 UTC
Thanks for the heads up about the release. I've got a CVE ID for this: CVE-2018-19134. I've asked them to reserve the details until the release is out. By the way, you can actually use this to execute command, but you need to know the address of the appropriate function to do so, so executing arbitrary command would probably still requires knowledge of the memory layout, which probably still requires other bugs (e.g. another information leak bug). e.g. with ASLR turned off in gdb:

gdb --args ./gs -q -sDEVICE=ppmraw -dSAFER -dDEBUG

p system
$1 = {int (const char *)} 0x7ffff65f7440 <__libc_system>

GS><</Implementation [[16#4141414141414141 [16#51 16#7ffff65f7440]]] >> setpattern
[New process 1726]
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
process 1726 is executing new program: /bin/dash
sh: 1: 
        : not found
Comment 9 Man Yue Mo 2018-11-13 11:57:37 UTC
I noticed that the affected code is duplicated in `patterncomponent`:

http://git.ghostscript.com/?p=ghostpdl.git;a=blob;f=psi/zcolor.c;h=3b8849ff33b11ef3ed2fb73dd009cf5cd06fd69f;hb=2dceb0400c5a571f23070891b8a8028d04926de1#l4805

This can be reached from `setcolor_cont`

http://git.ghostscript.com/?p=ghostpdl.git;a=blob;f=psi/zcolor.c;h=3b8849ff33b11ef3ed2fb73dd009cf5cd06fd69f;hb=2dceb0400c5a571f23070891b8a8028d04926de1#l6088

from `setpattern`. I am not totally sure whether the check that is already in place in the commit will also make this one safe, but it may be good just to add those checks in `patterncomponent`. Thanks.
Comment 10 Ken Sharp 2018-11-13 12:11:34 UTC
(In reply to Man Yue Mo from comment #9)
> I noticed that the affected code is duplicated in `patterncomponent`:
...
> from `setpattern`. I am not totally sure whether the check that is already
> in place in the commit will also make this one safe, but it may be good just
> to add those checks in `patterncomponent`. Thanks.

You can't get to setcolor_cont without first going through setcolor, if the check against the Implementation fails there then you'll never get to patterncomponent, if it succeeds, then its valid for patterncomponent too.

So yes, this fix already covers it. We'd prefer not to add extra checking where its not required, as this has a performance penalty.
Comment 11 Man Yue Mo 2018-11-22 20:06:48 UTC
Just a heads up, I managed to turn this bug into full arbitrary command execution, so please make sure distro maintainers are aware of it when deciding whether to back-port this fix or not. Thanks.
Comment 12 Cedric 2018-12-04 14:21:33 UTC
Hi all,
I am wondering if the issue is currently under embargo. My understanding is "probably not", since it's fixed in 9.26.
:) but I prefer get a confirmation first before publishing the existence of that bug (so far, I have not found any public info yet)