Swfdec - Apr 2008 - [Bug 15528] New: jpeg decoder allocation size overflows

http://bugs.freedesktop.org/show_bug.cgi?id=15528

           Summary: jpeg decoder allocation size overflows
           Product: swfdec
           Version: git
          Platform: x86 (IA32)
        OS/Version: Linux (All)
            Status: NEW
          Severity: normal
          Priority: medium
         Component: library
        AssignedTo: swfdec at lists.freedesktop.org
        ReportedBy: jpihlaja at cc.helsinki.fi
         QAContact: swfdec at lists.freedesktop.org


Created an attachment (id=15947)
 --> (http://bugs.freedesktop.org/attachment.cgi?id=15947)
Test jpegs

The two files cookiemon.jpg and wookiemon.jpg in the attached tar file trigger
allocation overflows on x86 and amd64.  Valgrind says:

[for wookiemon.jpg]
==4516== Invalid write of size 1
==4516==    at 0x445D8F8: (within /usr/lib/liboil-0.3.so.0.1.0)
==4516==    by 0x80497FF: jpeg_decoder_decode_entropy_segment (jpeg.c:503)
==4516==    by 0x8049DEB: jpeg_decoder_decode (jpeg.c:683)
==4516==    by 0x804B1E1: jpeg_decode_argb (jpeg_rgb_decoder.c:58)
==4516==    by 0x8048A51: main (load.c:46)
==4516==  Address 0x632C490 is 0 bytes after a block of size 0 alloc'd
==4516==    at 0x442438B: malloc (vg_replace_malloc.c:149)
==4516==    by 0x8049084: jpeg_decoder_init_decoder (jpeg.c:192)
==4516==    by 0x8049CD3: jpeg_decoder_decode (jpeg.c:654)
==4516==    by 0x804B1E1: jpeg_decode_argb (jpeg_rgb_decoder.c:58)
==4516==    by 0x8048A51: main (load.c:46)

[for cookiemon.jpg]
==4520== Invalid write of size 4
==4520==    at 0x804B470: yuv_mux (jpeg_rgb_decoder.c:103)
==4520==    by 0x804BDDF: get_argb_420 (jpeg_rgb_decoder.c:278)
==4520==    by 0x804B329: jpeg_decoder_get_argb_image (jpeg_rgb_decoder.c:89)
==4520==    by 0x804B217: jpeg_decode_argb (jpeg_rgb_decoder.c:63)
==4520==    by 0x8048A51: main (load.c:46)
==4520==  Address 0x78C57D80 is 0 bytes after a block of size 40 alloc'd
==4520==    at 0x442438B: malloc (vg_replace_malloc.c:149)
==4520==    by 0x804BB54: get_argb_420 (jpeg_rgb_decoder.c:253)
==4520==    by 0x804B329: jpeg_decoder_get_argb_image (jpeg_rgb_decoder.c:89)
==4520==    by 0x804B217: jpeg_decode_argb (jpeg_rgb_decoder.c:63)
==4520==    by 0x8048A51: main (load.c:46)


-- 
Configure bugmail: http://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.

http://bugs.freedesktop.org/show_bug.cgi?id=15528





--- Comment #1 from Riccardo Magliocchetti <riccardo at datahost.it> 
2008-09-06 12:09:51 PST ---
Created an attachment (id=18705)
 --> (http://bugs.freedesktop.org/attachment.cgi?id=18705)
don't segfault with wookiemon.jpeg

I think that when handling input using g_try_malloc is way better than
segfaulting.


-- 
Configure bugmail: http://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.

http://bugs.freedesktop.org/show_bug.cgi?id=15528





--- Comment #2 from Riccardo Magliocchetti <riccardo at datahost.it> 
2008-09-06 12:11:14 PST ---
Created an attachment (id=18706)
 --> (http://bugs.freedesktop.org/attachment.cgi?id=18706)
tentative of a test

This is the test i'm trying against.


-- 
Configure bugmail: http://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.

http://bugs.freedesktop.org/show_bug.cgi?id=15528


Riccardo Magliocchetti <riccardo at datahost.it> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #18705|0                           |1
        is obsolete|                            |




--- Comment #3 from Riccardo Magliocchetti <riccardo at datahost.it> 
2008-09-07 02:17:05 PST ---
Created an attachment (id=18720)
 --> (http://bugs.freedesktop.org/attachment.cgi?id=18720)
validate jpeg image size in the right place

This one fix both image loading for me.


-- 
Configure bugmail: http://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.

http://bugs.freedesktop.org/show_bug.cgi?id=15528





--- Comment #4 from Riccardo Magliocchetti <riccardo at datahost.it> 
2008-09-07 02:19:03 PST ---
Created an attachment (id=18721)
 --> (http://bugs.freedesktop.org/attachment.cgi?id=18721)
test for cookiemon


-- 
Configure bugmail: http://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.

http://bugs.freedesktop.org/show_bug.cgi?id=15528


Benjamin Otte <otte at gnome.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED




--- Comment #5 from Benjamin Otte <otte at gnome.org>  2008-09-07 09:26:13
PST ---
patch applied in git.


-- 
Configure bugmail: http://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.

http://bugs.freedesktop.org/show_bug.cgi?id=15528





--- Comment #6 from M Joonas Pihlaja <jpihlaja at cc.helsinki.fi> 
2008-09-15 08:36:38 PST ---
Created an attachment (id=18881)
 --> (http://bugs.freedesktop.org/attachment.cgi?id=18881)
avoid size validation

Hi,

I had some fun this weekend while looking at dsjpeg more closely.  The proposed
patch doesn't work 100% correctly, but does make crashing the decoder more
difficult.  Checking for multiplication overflow when computing a*b should
either take the form of a test like (a*b)/b = a or explicitly checking the
sizes of a and b to avoid overflow.  The attached test case will crash due to a
NULL pointer dereference (on 32 bit machines), or eventually due to an out of
bounds write (on 64 bit machines.)

On a 64 bit machine, the result in gdb is reproduced below. (Valgrind takes way
too long as the case needs to trawl through a lot of memory before hitting the
segfaulting overwrite.)

Program received signal SIGSEGV, Segmentation fault.
0x00002b2c733b6000 in oil_test_new () from /usr/lib/liboil-0.3.so.0
(gdb) up
#1  0x0000000000404962 in get_argb_420 (dec=0x507030) at jpeg_rgb_decoder.c:279
279         oil_colorspace_argb(argbp, tmp, jfif_matrix, dec->width);
(gdb) p argb
No symbol "argb" in current context.
(gdb) p argbp
$1 = (uint32_t *) 0x2b2d9e48a010
(gdb) p tmp
$2 = (uint32_t *) 0x2b2cf39bf010


-- 
Configure bugmail: http://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.

http://bugs.freedesktop.org/show_bug.cgi?id=15528





--- Comment #7 from M Joonas Pihlaja <jpihlaja at cc.helsinki.fi> 
2008-09-15 08:41:03 PST ---
Created an attachment (id=18883)
 --> (http://bugs.freedesktop.org/attachment.cgi?id=18883)
bypass check to hit another malloc

This test case causes a malloc argument overflow later in the code causing
malloc to return NULL and a subsequent NULL ptr deref on both x86 and x86_64.


-- 
Configure bugmail: http://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.

http://bugs.freedesktop.org/show_bug.cgi?id=15528





--- Comment #8 from Riccardo Magliocchetti <riccardo at datahost.it> 
2008-09-15 09:10:16 PST ---
Benjamin committed a different fix that is indeed more correct than mine.
I've
tested both your new attachments with latest git and both are handled
correctly. If you have other images please try them with 0.8.0 or git.


-- 
Configure bugmail: http://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.