Bug 1254242 (CVE-2015-5203)

Summary: CVE-2015-5203 jasper: integer overflow in jas_image_cmpt_create()
Product: [Other] Security Response Reporter: Martin Prpič <mprpic>
Component: vulnerabilityAssignee: Red Hat Product Security <security-response-team>
Status: CLOSED ERRATA QA Contact:
Severity: medium Docs Contact:
Priority: medium    
Version: unspecifiedCC: acathrow, alonbl, bazulay, bmcclain, carnil, cfergeau, dblechte, ecohen, eedri, erik-fedora, fabrice, idith, jpopelka, jridky, lsurette, mgoldboi, michal.skrivanek, mike, phracek, rbalakri, rdieter, rh-spice-bugs, rjones, sherold, srevivo, yeti, yeylon, ykaul, ylavi
Target Milestone: ---Keywords: Security
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: jasper 1.900.22 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2017-05-09 21:43:21 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 1254244, 1254245, 1254246, 1254247, 1260926, 1439171, 1439172, 1439173, 1439174    
Bug Blocks: 1254249, 1314477    
Attachments:
Description Flags
Proposed patch none

Description Martin Prpič 2015-08-17 13:47:29 UTC
A double free flaw was found in the way JasPer's jasper_image_stop_load() function parsed certain JPEG 2000 image files. A specially crafted file could cause an application using JasPer to crash.

Original report:

http://seclists.org/oss-sec/2015/q3/366

Comment 1 Martin Prpič 2015-08-17 13:48:23 UTC
Created mingw-jasper tracking bugs for this issue:

Affects: fedora-all [bug 1254245]
Affects: epel-7 [bug 1254247]

Comment 2 Martin Prpič 2015-08-17 13:48:28 UTC
Created jasper tracking bugs for this issue:

Affects: fedora-all [bug 1254244]
Affects: epel-5 [bug 1254246]

Comment 3 Tomas Hoger 2015-08-17 14:21:35 UTC
There are few problems here.  The first is in jas_image_cmpt_create() function in src/libjasper/base/jas_image.c which contains this code:

 310     long size;
 ...
 328     size = cmpt->width_ * cmpt->height_ * cmpt->cps_;
 329     cmpt->stream_ = (inmem) ? jas_stream_memopen(0, size) : jas_stream_tmpfile();

Computed size here can be 2^31 or more.  That's a problem for jas_stream_memopen().  Type of its second argument - bufsize - is int, so the function can see negative bufsize.  That's indication that a buffer that can be automatically grown later is to be allocated.  From src/libjasper/base/jas_stream.c:

 171 jas_stream_t *jas_stream_memopen(char *buf, int bufsize)
 172 {
 ...
 203     /* If the buffer size specified is nonpositive, then the buffer
 204     is allocated internally and automatically grown as needed. */
 205     if (bufsize <= 0) {
 206         obj->bufsize_ = 1024;
 207         obj->growable_ = 1;
 208     } else {
 209         obj->bufsize_ = bufsize;
 210         obj->growable_ = 0;
 211     }
 212     if (buf) {
 213         obj->buf_ = (unsigned char *) buf;
 214     } else {
 215         obj->buf_ = jas_malloc(obj->bufsize_);
 216         obj->myalloc_ = 1;
 217     }

Back in jas_image_cmpt_create(), the following code is called subsequently:

 335     /* Zero the component data.  This isn't necessary, but it is
 336     convenient for debugging purposes. */
 337     if (jas_stream_seek(cmpt->stream_, size - 1, SEEK_SET) < 0 ||
 338       jas_stream_putc(cmpt->stream_, 0) == EOF ||
 339       jas_stream_seek(cmpt->stream_, 0, SEEK_SET) < 0) {

There, jas_stream_flushbuf() is called which further leads to call to mem_write().  Again from src/libjasper/base/jas_stream.c:

1008     long newbufsize;
1009     long newpos;
1010 
1011     newpos = m->pos_ + cnt;
1012     if (newpos > m->bufsize_ && m->growable_) {
1013         newbufsize = m->bufsize_;
1014         while (newbufsize < newpos) {
1015             newbufsize <<= 1;
1016             assert(newbufsize >= 0);
1017         }
1018         if (mem_resize(m, newbufsize)) {

At this point, m->pos_ corresponds to the original size from jas_image_cmpt_create().  As seen above, initial bufsize_ is initially set to 1024 and doubled with every iteration of the while loop.  As newpos > 2^31, computed newbufsize is 2^32, which is passed to mem_resize().  The mem_resize() is wrapper around jas_realloc(), but its bufsize argument is int:

 990 static int mem_resize(jas_stream_memobj_t *m, int bufsize)
 ...
 995     if (!(buf = jas_realloc(m->buf_, bufsize))) {

This leads to jas_realloc() / realloc() called with the size of 0, which frees the buffer instead of increasing its size.  Subsequently called mem_close() (called via jas_image_cmpt_destroy() and jas_stream_close()) double frees the buffer.

The above notes are for 64bit systems where sizeof(long) > sizeof(int).

Comment 4 Adam Mariš 2015-08-25 11:10:00 UTC
Created attachment 1066820 [details]
Proposed patch

Patch was proposed at http://seclists.org/oss-sec/2015/q3/416

Comment 7 Fedora Update System 2016-08-15 21:21:50 UTC
jasper-1.900.1-33.fc24 has been pushed to the Fedora 24 stable repository. If problems still persist, please make note of it in this bug report.

Comment 8 Fedora Update System 2016-09-09 21:52:02 UTC
jasper-1.900.1-33.fc25 has been pushed to the Fedora 25 stable repository. If problems still persist, please make note of it in this bug report.

Comment 9 Fabrice Bellet 2016-09-13 18:37:23 UTC
Hi!

This patch breaks the API used by gdk-pixbuf, and for example makes nautilus abort:

https://git.gnome.org/browse/gdk-pixbuf/tree/gdk-pixbuf/io-jasper.c#n70

I have the same problem than this one (Fedora-24):

https://bugs.archlinux.org/task/46161

Comment 10 David Nečas 2016-09-15 08:44:43 UTC
Confirming that this jasper-libs-1.900.1-33.fc24 breaks gdk-pixbuf-based programs terribly.

The worst part is that gdk-pixbuf is not exactly excellent at file format detection.  So programs like geeqie started aborting even when you **do not have any JPEG 2000 files** because occasionally some random file gets misdetected and the jasper pixbuf loader invoked.

If you do not have any JPEG 2000 files the workaround is to remove the 

"/usr/lib64/gdk-pixbuf-2.0/2.10.0/loaders/libpixbufloader-jasper.so"
"jpeg2000" 4 "gdk-pixbuf" "JPEG 2000" "LGPL"
"image/jp2" "image/jpeg2000" "image/jpx" ""
"jp2" "jpc" "jpx" "j2k" "jpf" ""
"    jP" "!!!!  " 100
"\377O\377Q" "" 100

section from /usr/lib64/gdk-pixbuf-2.0/2.10.0/loaders.cache or just delete /usr/lib64/gdk-pixbuf-2.0/2.10.0/loaders/libpixbufloader-jasper.so outright and re-generate the cache.

Of course, this is overwritten when gdk-pixbuf2 (or librsvg2 or whatever has pixbuf loaders) is updated.  Wish the jasper pixbuf loader was a separate package...

Comment 11 Tomas Hoger 2016-12-07 21:56:35 UTC
(In reply to Tomas Hoger from comment #3)
> There are few problems here.  The first is in jas_image_cmpt_create()
> function in src/libjasper/base/jas_image.c which contains this code:
> 
>  310     long size;
>  ...
>  328     size = cmpt->width_ * cmpt->height_ * cmpt->cps_;
>  329     cmpt->stream_ = (inmem) ? jas_stream_memopen(0, size) :
> jas_stream_tmpfile();
> 
> Computed size here can be 2^31 or more.

There was an attempt to fix this in version 1.900.8 by adding integer overflow checks to the jas_image_cmpt_create() function:

https://github.com/mdadams/jasper/commit/b35a05635e56f554870ce85f64293a3868793f69

  if (!jas_safe_size_mul(cmpt->width_, cmpt->height_, &size) ||
    !jas_safe_size_mul(size, cmpt->cps_, &size)) {
  	goto error;
  }

This only ensures that the result of multiplication can fit into size_t without overflowing.  However, as size is passed to jas_stream_memopen(), which uses int type for its second argument bufsize, this check does not prevent the jas_stream_memopen() function from being called with negative bufsize as a result of the size_t -> int type conversion.

The code was further changed in version 1.900.25 to use one call of jas_safe_size_mul3() instead of two calls of jas_safe_size_mul() to implement the overflow check:

https://github.com/mdadams/jasper/commit/d42b2388f7f8e0332c846675133acea151fc557a#diff-6cf8c17c2a7595ef1481b68354696c37R379

  if (!jas_safe_size_mul3(cmpt->width_, cmpt->height_, cmpt->cps_, &size)) {
  	goto error;
  }

This version of the check has the same problem.

Another related check was added in version 1.900.25 via this commit:

https://github.com/mdadams/jasper/commit/d42b2388f7f8e0332c846675133acea151fc557a#diff-6cf8c17c2a7595ef1481b68354696c37R357

  if (!jas_safe_intfast32_mul3(width, height, depth, 0)) {
  	goto error;
  }

where:

  cmpt->width_ = width;
  cmpt->height_ = height;
  cmpt->cps_ = (depth + 7) / 8;

This check ensures that the multiplication to produce size fits into int_fast32_t.  However, int_fast32_t type is not guaranteed to be exactly 32 bits, it's only guaranteed to be at least 32 bits.  For example on x86_64 it is 64 bit type and hence it does not prevent overflow or truncation in call to jas_stream_memopen().

It seems the fix for this part of the problem was completed via this commit added in version 1.900.22:

https://github.com/mdadams/jasper/commit/634ce8e8a5accc0fa05dd2c20d42b4749d4b2735

It uses similar approach to what was used by the problematic patch attached in comment 4 and changes types of certain variables from int to size_t.  However, it does not change API of the jas_stream_memopen() function, it rather adds new function jas_stream_memopen2(), which uses size_t type for the bufsize argument.  jas_stream_memopen() is implemented as wrapper around jas_stream_memopen2().  The jas_image_cmpt_create() function was changed to call jas_stream_memopen2(), removing size_t -> int cast.

If we decide to backport the fix without introducing jas_stream_memopen2(), alternative should be to add overflow check guarding computation of size, and next ensure that size does not exceed INT_MAX before calling jas_stream_memopen().

> At this point, m->pos_ corresponds to the original size from
> jas_image_cmpt_create().  As seen above, initial bufsize_ is initially set
> to 1024 and doubled with every iteration of the while loop.  As newpos >
> 2^31, computed newbufsize is 2^32, which is passed to mem_resize().  The
> mem_resize() is wrapper around jas_realloc(), but its bufsize argument is
> int:
> 
>  990 static int mem_resize(jas_stream_memobj_t *m, int bufsize)
>  ...
>  995     if (!(buf = jas_realloc(m->buf_, bufsize))) {
> 
> This leads to jas_realloc() / realloc() called with the size of 0, which
> frees the buffer instead of increasing its size.  Subsequently called
> mem_close() (called via jas_image_cmpt_destroy() and jas_stream_close())
> double frees the buffer.

This double free issue got separate CVE id CVE-2016-8693 and was fixed in version 1.900.10, see bug 1385507.

Comment 13 Tomas Hoger 2017-03-24 21:51:07 UTC
As the double-free that is a symptom of this issue was addressed via a spearate CVE-2016-8693, the only problem left under this CVE is the integer overflow problem in the jas_image_cmpt_create() function.  As noted above, that problem was fixed in upstream version 1.900.22 via introduction of the new API jas_stream_memopen2().  Such change is problematic to backport.  The less invasive fix is to ensure that cmpt->width_ * cmpt->height_ * cmpt->cps_ does not exceed INT_MAX.

Comment 14 Tomas Hoger 2017-03-29 13:37:55 UTC
(In reply to Tomas Hoger from comment #11)
> There was an attempt to fix this in version 1.900.8 by adding integer
> overflow checks to the jas_image_cmpt_create() function:
> 
> https://github.com/mdadams/jasper/commit/b35a05635e56f554870ce85f64293a3868793f69

...

> This only ensures that the result of multiplication can fit into size_t
> without overflowing.  However, as size is passed to jas_stream_memopen(),
> which uses int type for its second argument bufsize, this check does not
> prevent the jas_stream_memopen() function from being called with negative
> bufsize as a result of the size_t -> int type conversion.

...

> It seems the fix for this part of the problem was completed via this commit
> added in version 1.900.22:
> 
> https://github.com/mdadams/jasper/commit/
> 634ce8e8a5accc0fa05dd2c20d42b4749d4b2735
> 
> It uses similar approach to what was used by the problematic patch attached
> in comment 4 and changes types of certain variables from int to size_t. 
> However, it does not change API of the jas_stream_memopen() function, it
> rather adds new function jas_stream_memopen2(), which uses size_t type for
> the bufsize argument.  jas_stream_memopen() is implemented as wrapper around
> jas_stream_memopen2().  The jas_image_cmpt_create() function was changed to
> call jas_stream_memopen2(), removing size_t -> int cast.

Note that this additional fix in 1.900.22 got another CVE assigned - CVE-2016-9262, see bug 1393882.

Comment 15 Tomas Hoger 2017-04-05 10:43:55 UTC
Acknowledgments:

Name: Gustavo Grieco

Comment 17 errata-xmlrpc 2017-05-09 17:15:19 UTC
This issue has been addressed in the following products:

  Red Hat Enterprise Linux 6
  Red Hat Enterprise Linux 7

Via RHSA-2017:1208 https://access.redhat.com/errata/RHSA-2017:1208