Created mingw-jasper tracking bugs for this issue:

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

Created jasper tracking bugs for this issue:

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

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).

Created attachment 1066820 [details]
Proposed patch

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

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.

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.

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

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...

(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.

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.

(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.

Acknowledgments:

Name: Gustavo Grieco

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