Skip to content

Commit 3bf5edd

Browse files
wiredfoolhugovk
authored andcommitted
Fix OOB Read in Jpeg2KDecode CVE-2021-25287,CVE-2021-25288
* 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
1 parent 8ec0278 commit 3bf5edd

6 files changed

+41
-8
lines changed
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.

Tests/test_file_jpeg2k.py

+16
Original file line numberDiff line numberDiff line change
@@ -231,3 +231,19 @@ def test_parser_feed():
231231

232232
# Assert
233233
assert p.image.size == (640, 480)
234+
235+
236+
@pytest.mark.parametrize(
237+
"test_file",
238+
[
239+
"Tests/images/crash-4fb027452e6988530aa5dabee76eecacb3b79f8a.j2k",
240+
"Tests/images/crash-7d4c83eb92150fb8f1653a697703ae06ae7c4998.j2k",
241+
"Tests/images/crash-ccca68ff40171fdae983d924e127a721cab2bd50.j2k",
242+
"Tests/images/crash-d2c93af851d3ab9a19e34503626368b2ecde9c03.j2k",
243+
],
244+
)
245+
def test_crashes(test_file):
246+
with open(test_file, "rb") as f:
247+
with Image.open(f) as im:
248+
# Valgrind should not complain here
249+
im.load()

src/libImaging/Jpeg2KDecode.c

+25-8
Original file line numberDiff line numberDiff line change
@@ -644,7 +644,7 @@ j2k_decode_entry(Imaging im, ImagingCodecState state) {
644644
j2k_unpacker_t unpack = NULL;
645645
size_t buffer_size = 0, tile_bytes = 0;
646646
unsigned n, tile_height, tile_width;
647-
int components;
647+
int total_component_width = 0;
648648

649649
stream = opj_stream_create(BUFFER_SIZE, OPJ_TRUE);
650650

@@ -814,23 +814,40 @@ j2k_decode_entry(Imaging im, ImagingCodecState state) {
814814
goto quick_exit;
815815
}
816816

817+
if (tile_info.nb_comps != image->numcomps) {
818+
state->errcode = IMAGING_CODEC_BROKEN;
819+
state->state = J2K_STATE_FAILED;
820+
goto quick_exit;
821+
}
822+
817823
/* Sometimes the tile_info.datasize we get back from openjpeg
818-
is less than numcomps*w*h, and we overflow in the
824+
is less than sum(comp_bytes)*w*h, and we overflow in the
819825
shuffle stage */
820826

821827
tile_width = tile_info.x1 - tile_info.x0;
822828
tile_height = tile_info.y1 - tile_info.y0;
823-
components = tile_info.nb_comps == 3 ? 4 : tile_info.nb_comps;
824-
if ((tile_width > UINT_MAX / components) ||
825-
(tile_height > UINT_MAX / components) ||
826-
(tile_width > UINT_MAX / (tile_height * components)) ||
827-
(tile_height > UINT_MAX / (tile_width * components))) {
829+
830+
/* Total component width = sum (component_width) e.g, it's
831+
legal for an la file to have a 1 byte width for l, and 4 for
832+
a. and then a malicious file could have a smaller tile_bytes
833+
*/
834+
835+
for (n=0; n < tile_info.nb_comps; n++) {
836+
// see csize /acsize calcs
837+
int csize = (image->comps[n].prec + 7) >> 3;
838+
csize = (csize == 3) ? 4 : csize;
839+
total_component_width += csize;
840+
}
841+
if ((tile_width > UINT_MAX / total_component_width) ||
842+
(tile_height > UINT_MAX / total_component_width) ||
843+
(tile_width > UINT_MAX / (tile_height * total_component_width)) ||
844+
(tile_height > UINT_MAX / (tile_width * total_component_width))) {
828845
state->errcode = IMAGING_CODEC_BROKEN;
829846
state->state = J2K_STATE_FAILED;
830847
goto quick_exit;
831848
}
832849

833-
tile_bytes = tile_width * tile_height * components;
850+
tile_bytes = tile_width * tile_height * total_component_width;
834851

835852
if (tile_bytes > tile_info.data_size) {
836853
tile_info.data_size = tile_bytes;

0 commit comments

Comments
 (0)