From cf6da6b79080a8c16984102fdc85f7ce28dca613 Mon Sep 17 00:00:00 2001 From: Eric Soroos Date: Mon, 9 Mar 2020 22:09:49 +0000 Subject: [PATCH 1/4] Fix for OOB Read in DecodeJpeg2k --- src/libImaging/Jpeg2KDecode.c | 60 +++++++++++++++++++++++++---------- 1 file changed, 43 insertions(+), 17 deletions(-) diff --git a/src/libImaging/Jpeg2KDecode.c b/src/libImaging/Jpeg2KDecode.c index f2e437dda28..6cf8b8e9c51 100644 --- a/src/libImaging/Jpeg2KDecode.c +++ b/src/libImaging/Jpeg2KDecode.c @@ -110,6 +110,7 @@ j2ku_gray_l(opj_image_t *in, const JPEG2KTILEINFO *tileinfo, if (shift < 0) offset += 1 << (-shift - 1); + /* csiz*h*w + offset = tileinfo.datasize */ switch (csiz) { case 1: for (y = 0; y < h; ++y) { @@ -557,8 +558,10 @@ j2k_decode_entry(Imaging im, ImagingCodecState state) opj_dparameters_t params; OPJ_COLOR_SPACE color_space; j2k_unpacker_t unpack = NULL; - size_t buffer_size = 0; - unsigned n; + size_t buffer_size = 0, tile_bytes = 0; + unsigned n, tile_height, tile_width; + int components; + stream = opj_stream_create(BUFFER_SIZE, OPJ_TRUE); @@ -703,8 +706,44 @@ j2k_decode_entry(Imaging im, ImagingCodecState state) tile_info.x1 = (tile_info.x1 + correction) >> context->reduce; tile_info.y1 = (tile_info.y1 + correction) >> context->reduce; + /* Check the tile bounds; if the tile is outside the image area, + or if it has a negative width or height (i.e. the coordinates are + swapped), bail. */ + if (tile_info.x0 >= tile_info.x1 + || tile_info.y0 >= tile_info.y1 + || tile_info.x0 < image->x0 + || tile_info.y0 < image->y0 + || tile_info.x1 - image->x0 > im->xsize + || tile_info.y1 - image->y0 > im->ysize) { + state->errcode = IMAGING_CODEC_BROKEN; + state->state = J2K_STATE_FAILED; + goto quick_exit; + } + + /* Sometimes the tile_info.datasize we get back from openjpeg + is is less than numcomps*w*h, and we overflow in the + shuffle stage */ + + tile_width = tile_info.x1 - tile_info.x0; + tile_height = tile_info.y1 - tile_info.y0; + components = tile_info.nb_comps == 3 ? 4 : tile_info.nb_comps; + if (( tile_width > UINT_MAX / components ) || + ( tile_height > UINT_MAX / components ) || + ( tile_width > UINT_MAX / (tile_height * components )) || + ( tile_height > UINT_MAX / (tile_width * components ))) { + state->errcode = IMAGING_CODEC_BROKEN; + state->state = J2K_STATE_FAILED; + goto quick_exit; + } + + tile_bytes = tile_width * tile_height * components; + + if (tile_bytes > tile_info.data_size) { + tile_info.data_size = tile_bytes; + } + if (buffer_size < tile_info.data_size) { - /* malloc check ok, tile_info.data_size from openjpeg */ + /* malloc check ok, overflow and tile size sanity check above */ UINT8 *new = realloc (state->buffer, tile_info.data_size); if (!new) { state->errcode = IMAGING_CODEC_MEMORY; @@ -715,6 +754,7 @@ j2k_decode_entry(Imaging im, ImagingCodecState state) buffer_size = tile_info.data_size; } + if (!opj_decode_tile_data(codec, tile_info.tile_index, (OPJ_BYTE *)state->buffer, @@ -725,20 +765,6 @@ j2k_decode_entry(Imaging im, ImagingCodecState state) goto quick_exit; } - /* Check the tile bounds; if the tile is outside the image area, - or if it has a negative width or height (i.e. the coordinates are - swapped), bail. */ - if (tile_info.x0 >= tile_info.x1 - || tile_info.y0 >= tile_info.y1 - || tile_info.x0 < image->x0 - || tile_info.y0 < image->y0 - || tile_info.x1 - image->x0 > im->xsize - || tile_info.y1 - image->y0 > im->ysize) { - state->errcode = IMAGING_CODEC_BROKEN; - state->state = J2K_STATE_FAILED; - goto quick_exit; - } - unpack(image, &tile_info, state->buffer, im); } From 30443d39bd932159bbb66f88df9f34fec2100a4f Mon Sep 17 00:00:00 2001 From: Eric Soroos Date: Mon, 9 Mar 2020 22:12:00 +0000 Subject: [PATCH 2/4] Tests for jp2 overflow --- Tests/check_jp2_overflow.py | 29 +++++++++++++++++++++++++++++ Tests/images/00r0_gray_l.jp2 | Bin 0 -> 614 bytes Tests/images/00r1_graya_la.jp2 | Bin 0 -> 335 bytes 3 files changed, 29 insertions(+) create mode 100755 Tests/check_jp2_overflow.py create mode 100644 Tests/images/00r0_gray_l.jp2 create mode 100644 Tests/images/00r1_graya_la.jp2 diff --git a/Tests/check_jp2_overflow.py b/Tests/check_jp2_overflow.py new file mode 100755 index 00000000000..920474c8172 --- /dev/null +++ b/Tests/check_jp2_overflow.py @@ -0,0 +1,29 @@ +#!/usr/bin/env python + +# Reproductions/tests for OOB read errors in FliDecode.c + +# When run in python, all of these images should fail for +# one reason or another, either as a buffer overrun, +# unrecognized datastream, or truncated image file. +# There shouldn't be any segfaults. +# +# if run like +# `valgrind --tool=memcheck python check_jp2_overflow.py 2>&1 | grep Decode.c` +# the output should be empty. There may be python issues +# in the valgrind especially if run in a debug python +# version. + + +from PIL import Image + +repro = ('00r0_gray_l.jp2', '00r1_graya_la.jp2' + ) + +for path in repro: + im = Image.open(path) + try: + im.load() + except Exception as msg: + print(msg) + + diff --git a/Tests/images/00r0_gray_l.jp2 b/Tests/images/00r0_gray_l.jp2 new file mode 100644 index 0000000000000000000000000000000000000000..28612238a9cae1a8eca050d3df28f4119624a876 GIT binary patch literal 614 zcmZQzVBpCLP*C9IYUg5LU=T?wsVvAUFj4@r8KAT-kj?;d#WFKeihwjP5c@DNva>S+ z84Uc%`8h?53_3;W#R@>KP*G}einmw;0|>LQKtv>ykww@+1}Fdb{~ySp36uvJ1~C-O z;Xva9E#hVTAH=`|lx1XK1sVB2hCx}WT&|pbhw7 z2D@tNM;pZ%nrT6Vd^D0OOJ?Ln6PlM4?kqYC#d0hz;(MCCPJSWIA#-*h!V zfBW(725#SZL1z~y>k3@z*m#hY`{c~y{fpfA_MGpu`O$NHCQtE-O)H=2HBL4&$^Uj( z?q-J8WoNt2jen*qTgH6sD_1-D_|PA#wJCc3$6pAncj{1lZ+Ib=V}r)BRWUo|-G1^5 z3O}9O>~QGhalX{^#(R`l{Iw!?9LZW_*ZuVPmWbC6M1P(<`g>}U%K1rxmZgz5d6Mh0 z%hNA=AL()Jc(`z%`|G@f_Wve%o7Nx7n$f@g##ZMaGHMdqa!Up7{7z;$`)!Frr~c7H oye`)!t6uyOq2cINvQ6RbT=6iylrom<_X^UMt&U@A-249~0D?o$$^ZZW literal 0 HcmV?d00001 diff --git a/Tests/images/00r1_graya_la.jp2 b/Tests/images/00r1_graya_la.jp2 new file mode 100644 index 0000000000000000000000000000000000000000..f3f840a08e378fbdaf8cdc5b231ea6862f68ec06 GIT binary patch literal 335 zcmZQzVBpCLP*C9IYUg5LU=T?wsVvAUFj4@r8KAT?kj?;d#WFKeih#5N7&Ec6GXfb5 z{K@$_MPL?#AdoAToRXTxzyy*30!APN1E3Mf|NZ|5GU$N03P2KsVL$=M0AUDVVsB(* zXJq^z#J~d-U}RurVPRnWAHyK5;1LiI0Yd>E9s&PTfR-|PCuS$6lKW-7{-3}A z@(2S%4N%YI<5yT1X8;L~faMQZ*gl^>z`_$>WlejaJCOIozHBX{G&^n;(f8O#>WT$jAqG~@M=OACIoFwC8{l95GQ Z;KspEJPh&6k9REK-H`Btt@iQ%n*b)BSDpX> literal 0 HcmV?d00001 From 6e86d235c8fc995d4cca9e4a79044e82810465a3 Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 26 Mar 2020 21:41:06 +0200 Subject: [PATCH 3/4] Format with Black --- Tests/check_jp2_overflow.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/Tests/check_jp2_overflow.py b/Tests/check_jp2_overflow.py index 920474c8172..a7a343c98ec 100755 --- a/Tests/check_jp2_overflow.py +++ b/Tests/check_jp2_overflow.py @@ -11,13 +11,12 @@ # `valgrind --tool=memcheck python check_jp2_overflow.py 2>&1 | grep Decode.c` # the output should be empty. There may be python issues # in the valgrind especially if run in a debug python -# version. +# version. from PIL import Image -repro = ('00r0_gray_l.jp2', '00r1_graya_la.jp2' - ) +repro = ("00r0_gray_l.jp2", "00r1_graya_la.jp2") for path in repro: im = Image.open(path) @@ -25,5 +24,3 @@ im.load() except Exception as msg: print(msg) - - From c5e9de15b1b5e082626c68d443098ded36a15fc8 Mon Sep 17 00:00:00 2001 From: Hugo van Kemenade Date: Tue, 31 Mar 2020 11:09:32 +0300 Subject: [PATCH 4/4] Fix typo --- src/libImaging/Jpeg2KDecode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libImaging/Jpeg2KDecode.c b/src/libImaging/Jpeg2KDecode.c index 6cf8b8e9c51..d304511d1a9 100644 --- a/src/libImaging/Jpeg2KDecode.c +++ b/src/libImaging/Jpeg2KDecode.c @@ -721,7 +721,7 @@ j2k_decode_entry(Imaging im, ImagingCodecState state) } /* Sometimes the tile_info.datasize we get back from openjpeg - is is less than numcomps*w*h, and we overflow in the + is less than numcomps*w*h, and we overflow in the shuffle stage */ tile_width = tile_info.x1 - tile_info.x0;