Bug 697178

Summary:	/OutputICCProfile .putdeviceparams -dSAFER bypass
Product:	Ghostscript	Reporter:	Florian Weimer <fw>
Component:	PS Interpreter	Assignee:	Chris Liddell (chrisl) <chris.liddell>
Status:	RESOLVED FIXED
Severity:	normal	CC:	chris.liddell, omarandemad, samuel.damashek, taviso
Priority:	P4
Version:	master
Hardware:	PC
OS:	Linux
Customer:		Word Size:	---
Attachments:	putdeviceparams.ps

Description Florian Weimer 2016-09-30 13:40:06 UTC

Created attachment 12981 [details]
putdeviceparams.ps

Tavis Ormandy reported another -dSAFER bypass:

  http://www.openwall.com/lists/oss-security/2016/09/30/8

commit 1fae53a708fca6c2ac0417bc23f5d095cc379250
Author: Chris Liddell <chris.liddell@artifex.com>
Date:   Thu Jul 30 17:27:23 2015 +0100

    Bug 696101: fix uses of the sfopen API.
    
    The stream API in GS is defined as *always* opening files in
    binary mode, where applicable, so there is no need for the API
    clients to specify binary mode.
    
    This is previously been benign, and thus ignored, but reportedly
    ending up with a duplicate 'b' character in the mode causes a
    crash on Windows 10.
    
    No cluster differences.

I'm attaching the reproducer from the oss-security posting.

Before the commit, the file is only opened, with potential directory traversal, so this should be fixed as well, but the impact is greatly reduced.

Comment 1 Florian Weimer 2016-09-30 14:12:29 UTC

'b' in the mode argument causes glibc's popen to fail, which is why this commit exposed the code execution bug on GNU/Linux.  For other systems, there may be code execution even before this change.

Comment 2 Chris Liddell (chrisl) 2016-09-30 17:23:55 UTC

I'm baffled about what you are reporting.

What is the attack vector here - bearing in mind that the parameter being set is supposed to be an ICC profile file, and something that *isn't* a valid ICC profile will cause an error to occur.

Comment 3 Tavis Ormandy 2016-09-30 18:08:18 UTC

The problem is that this entirely defeats -dSAFER, and permits arbitrary command execution in any application that uses -dSAFER to process untrusted documents (ImageMagick, cups, gimp, evince and dozens of other automated document processing systems).

https://codesearch.debian.net/search?q=-dSAFER&perpkg=1

The attached reproducer works perfectly for me every time with gs -dSAFER in 9.18 and higher?

Comment 4 Chris Liddell (chrisl) 2016-09-30 19:14:38 UTC

(In reply to Tavis Ormandy from comment #3)
> The problem is that this entirely defeats -dSAFER, and permits arbitrary
> command execution 

I don't see how that's the case. An ICC profile is just data, it is not "interpreted" in the way that Postscript is.

Comment 5 Tavis Ormandy 2016-09-30 20:53:15 UTC

It's the pathname that's the problem, not the data.

You can see in the example that %pipe% is used and that's interpreted as a special device (like %stdin%, %null%, etc).

Can you repro the testcase? We're in agreement that -dSAFER should prevent an untrusted file from being able to run shell commands, right?

Comment 6 Chris Liddell (chrisl) 2016-10-01 04:12:34 UTC

Yeh, I see what you're getting at, and yes, I agree it's a problem.

This is going to take some thought as this is done at a rather lower level than SAFER is currently handled. This is beyond the scope of what SAFER was originally conceived to protect.

Comment 7 Tavis Ormandy 2016-10-01 08:00:07 UTC

As Florian points out, it might also be necessary to have ICC profiles checked against PermitFileReading, because you can set an arbitrary directory like this:

<< (ICCProfilesDir) (whatever) >> .setuserparams

Just thought I'd mention it while you're thinking about it :-)

Comment 8 Chris Liddell (chrisl) 2016-10-05 08:49:30 UTC

Fixed in:
http://git.ghostscript.com/?p=user/chrisl/ghostpdl.git;a=commitdiff;h=71ac874