Bug 697178 - /OutputICCProfile .putdeviceparams -dSAFER bypass
Summary: /OutputICCProfile .putdeviceparams -dSAFER bypass
Status: RESOLVED FIXED
Alias: None
Product: Ghostscript
Classification: Unclassified
Component: PS Interpreter (show other bugs)
Version: master
Hardware: PC Linux
: P4 normal
Assignee: Chris Liddell (chrisl)
QA Contact: Bug traffic
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-09-30 13:40 UTC by Florian Weimer
Modified: 2019-07-22 07:12 UTC (History)
4 users (show)

See Also:
Customer:
Word Size: ---


Attachments
putdeviceparams.ps (98 bytes, text/plain)
2016-09-30 13:40 UTC, Florian Weimer
Details

Note You need to log in before you can comment on or make changes to this bug.
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