Summary: | Potential security issue (malloc smaller than requested) | ||
---|---|---|---|
Product: | Ghostscript | Reporter: | Stefan Cornelius <scorneli> |
Component: | General | Assignee: | Chris Liddell (chrisl) <chris.liddell> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | chris.liddell, joseph.heenan, william.robinet |
Priority: | P4 | ||
Version: | master | ||
Hardware: | PC | ||
OS: | Linux | ||
Customer: | Word Size: | --- | |
Attachments: |
test file
gdb session full backtrace |
Description
Stefan Cornelius
2015-07-01 12:14:58 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. 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. (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. Created attachment 11776 [details]
test file
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. 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"); (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. 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.
Created attachment 11782 [details]
full backtrace
Backtrace when hitting gs_heap_alloc_bytes with the huge size
*** Bug 696041 has been marked as a duplicate of this bug. *** 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) 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? 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. |