Skip to content

Commit

Permalink
Fix OOB Read in Jpeg2KDecode CVE-2021-25287,CVE-2021-25288
Browse files Browse the repository at this point in the history
* For J2k images with multiple bands, it's legal in to have different
  widths for each band, e.g. 1 byte for L, 4 bytes for A
* This dates to Pillow 2.4.0
  • Loading branch information
wiredfool authored and hugovk committed Apr 1, 2021
1 parent 8ec0278 commit 3bf5edd
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 8 deletions.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
16 changes: 16 additions & 0 deletions Tests/test_file_jpeg2k.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,3 +231,19 @@ def test_parser_feed():

# Assert
assert p.image.size == (640, 480)


@pytest.mark.parametrize(
"test_file",
[
"Tests/images/crash-4fb027452e6988530aa5dabee76eecacb3b79f8a.j2k",
"Tests/images/crash-7d4c83eb92150fb8f1653a697703ae06ae7c4998.j2k",
"Tests/images/crash-ccca68ff40171fdae983d924e127a721cab2bd50.j2k",
"Tests/images/crash-d2c93af851d3ab9a19e34503626368b2ecde9c03.j2k",
],
)
def test_crashes(test_file):
with open(test_file, "rb") as f:
with Image.open(f) as im:
# Valgrind should not complain here
im.load()
33 changes: 25 additions & 8 deletions src/libImaging/Jpeg2KDecode.c
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,7 @@ j2k_decode_entry(Imaging im, ImagingCodecState state) {
j2k_unpacker_t unpack = NULL;
size_t buffer_size = 0, tile_bytes = 0;
unsigned n, tile_height, tile_width;
int components;
int total_component_width = 0;

stream = opj_stream_create(BUFFER_SIZE, OPJ_TRUE);

Expand Down Expand Up @@ -814,23 +814,40 @@ j2k_decode_entry(Imaging im, ImagingCodecState state) {
goto quick_exit;
}

if (tile_info.nb_comps != image->numcomps) {
state->errcode = IMAGING_CODEC_BROKEN;
state->state = J2K_STATE_FAILED;
goto quick_exit;
}

/* Sometimes the tile_info.datasize we get back from openjpeg
is less than numcomps*w*h, and we overflow in the
is less than sum(comp_bytes)*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))) {

/* Total component width = sum (component_width) e.g, it's
legal for an la file to have a 1 byte width for l, and 4 for
a. and then a malicious file could have a smaller tile_bytes
*/

for (n=0; n < tile_info.nb_comps; n++) {
// see csize /acsize calcs
int csize = (image->comps[n].prec + 7) >> 3;
csize = (csize == 3) ? 4 : csize;
total_component_width += csize;
}
if ((tile_width > UINT_MAX / total_component_width) ||
(tile_height > UINT_MAX / total_component_width) ||
(tile_width > UINT_MAX / (tile_height * total_component_width)) ||
(tile_height > UINT_MAX / (tile_width * total_component_width))) {
state->errcode = IMAGING_CODEC_BROKEN;
state->state = J2K_STATE_FAILED;
goto quick_exit;
}

tile_bytes = tile_width * tile_height * components;
tile_bytes = tile_width * tile_height * total_component_width;

if (tile_bytes > tile_info.data_size) {
tile_info.data_size = tile_bytes;
Expand Down

0 comments on commit 3bf5edd

Please sign in to comment.