Bug 696070 - Potential security issue (malloc smaller than requested)
Summary: Potential security issue (malloc smaller than requested)
Status: RESOLVED FIXED
Alias: None
Product: Ghostscript
Classification: Unclassified
Component: General (show other bugs)
Version: master
Hardware: PC Linux
: P4 normal
Assignee: Chris Liddell (chrisl)
QA Contact: Bug traffic
URL:
Keywords:
: 696041 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-07-01 12:14 UTC by Stefan Cornelius
Modified: 2015-07-23 05:27 UTC (History)
3 users (show)

See Also:
Customer:
Word Size: ---


Attachments
test file (9 bytes, application/octet-stream)
2015-07-01 14:22 UTC, Stefan Cornelius
Details
gdb session (4.11 KB, text/plain)
2015-07-07 08:30 UTC, Stefan Cornelius
Details
full backtrace (22.58 KB, text/plain)
2015-07-07 08:42 UTC, Stefan Cornelius
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Stefan Cornelius 2015-07-01 12:14:58 UTC
Not adding anything until it's marked private
Comment 1 Stefan Cornelius 2015-07-01 12:34:10 UTC
Recently, somebody filed bug #696041. I don't have access to the bug, but I know that it is closed and have a rough idea about the content (I'm working for Red Hat Product Security and the reporter also contacted us).

The reporter likely also mentioned that http://git.ghostscript.com/?p=ghostpdl.git;a=commit;h=ecc7a199e9307475c37fea0c44d24b85df814ead fixed the issue.

I believe that this was an accidental fix; fixing the vector, but not the root cause.

I believe that the root issue is actually an integer overflow in one of
the memory allocation routines.

gs_heap_alloc_bytes() function in base/gsmalloc.c:
>        uint added = size + sizeof(gs_malloc_block_t);
>
>        if (mmem->limit - added < mmem->used)
>            set_msg("exceeded limit");
>        else if ((ptr = (byte *) Memento_label(malloc(added), cname)) == 0)
>            set_msg("failed");


If "size" is very large, then size+sizeof(gs_malloc_block_t) can cause
a wrap-around and "size" will suddenly be a lot smaller than it
actually needs to be.
Comment 2 Ray Johnston 2015-07-01 13:52:53 UTC
Note that we cannot work on this bug without a sample file attached and a
command line and/or procedure to reproduce the problem.

Note that this bug is now Private since I was denied access before logging
in as Artifex staff.
Comment 3 Stefan Cornelius 2015-07-01 14:20:58 UTC
(In reply to Ray Johnston from comment #2)
> Note that we cannot work on this bug without a sample file attached and a
> command line and/or procedure to reproduce the problem.
> 
> Note that this bug is now Private since I was denied access before logging
> in as Artifex staff.

I'll attach a test.ps in a minute.

To reproduce:
/usr/bin/gs -P- -dSAFER -dCompatibilityLevel=1.4 -q -P- -dNOPAUSE -dBATCH -sDEVICE=pdfwrite -sstdout=%stderr -sOutputFile=test.pdf -P- -dSAFER -dCompatibilityLevel=1.4 -c .setpdfwrite -f ./test.ps

Note that you need a ghostscript version with commit ecc7a199e9307475c37fea0c44d24b85df814ead reverted. This commit hides the vector currently known, but there may be other, unknown vectors. So I don't think that this commit is a proper fix.
Comment 4 Stefan Cornelius 2015-07-01 14:22:08 UTC
Created attachment 11776 [details]
test file
Comment 5 Ken Sharp 2015-07-07 07:45:32 UTC
You haven't said whether this is a 32 or 64-bit build. Rather than specifying a commit to remove it would probably be more useful to specify the SHA of a known non-working commit.

Did you build Ghostscript from our sources, or include some modifications for Red Hat (eg system shared libraries) ?

Your command line looks suspicious, you have specified -dSAFER twice and -P- three times, as well as "-c .setpdfwrite -f" which has not been required for some time.

I checked out commit 4c27c7d63, the one immediately before commit ecc7a199, built and tested with the supplied command line (verbatim, but modified for paths, obviously). Both the debug and release builds exit (not a seg fault) with an unrecoverable error, which seems reasonable to me, given that the input is gibberish.
Comment 6 Chris Liddell (chrisl) 2015-07-07 08:04:22 UTC
FWIW, I don't see a huge problem with a safety check thus:

        uint added = size + sizeof(gs_malloc_block_t);

        if (added <= size || mmem->limit - added < mmem->used)
            set_msg("exceeded limit");
        else if ((ptr = (byte *) Memento_label(malloc(added), cname)) == 0)
            set_msg("failed");
Comment 7 Ken Sharp 2015-07-07 08:11:24 UTC
(In reply to Chris Liddell (chrisl) from comment #6)
> FWIW, I don't see a huge problem with a safety check thus:
> 
>         uint added = size + sizeof(gs_malloc_block_t);
> 
>         if (added <= size || mmem->limit - added < mmem->used)
>             set_msg("exceeded limit");
>         else if ((ptr = (byte *) Memento_label(malloc(added), cname)) == 0)
>             set_msg("failed");

I don't have a problem with the check, but I'd like to see the error manifested.
Comment 8 Stefan Cornelius 2015-07-07 08:30:10 UTC
Created attachment 11781 [details]
gdb session

This is a gdb session, running on RHEL7.1 64bit with the 64bit version of RHELs ghostscript package. It probably has several differences from upstream, but I don't think any of them really matter, except the commit I've mentioned in my previous comments.
Comment 9 Stefan Cornelius 2015-07-07 08:42:41 UTC
Created attachment 11782 [details]
full backtrace

Backtrace when hitting gs_heap_alloc_bytes with the huge size
Comment 10 Chris Liddell (chrisl) 2015-07-08 01:14:19 UTC
*** Bug 696041 has been marked as a duplicate of this bug. ***
Comment 11 Chris Liddell (chrisl) 2015-07-08 01:18:15 UTC
The problem turns out to be that the binary in the file (either by accident or design) ends up as Postscript binary tokens attempting to allocate an *immense* array, hence the attempt to allocated 4Gb of memory, and hence the 32bit value overflowing when we try to add the memory manager housekeeping space to the allocation.


Fixed in:

http://git.ghostscript.com/?p=ghostpdl.git;a=commitdiff;h=0c0b0859

(Note that I accidentally referenced the original bug number 696041 hence I made it a duplicate)
Comment 12 Stefan Cornelius 2015-07-08 02:43:24 UTC
FYI, at Red Hat we've assigned this CVE-2015-3228 some time ago (back around the time when bug #696041 was filed, not sure if the CVE was communicated in this bug). Just like the two bugs here, the Red Hat bug is still private. Do you have any specific date in mind when this will be opened to the public? I'm OK with disclosing this immediately, not sure if you are, too?
Comment 13 Chris Liddell (chrisl) 2015-07-08 04:50:16 UTC
It's sort of "public" since the fix was committed to our git repo.

TBH, I don't plan to make this bug thread private at all, mainly because the ideal time would be at the time of the next release (September), but I'll never remember to do it in 2/3 month's time.....

If you want to remind me, I'll do it.