From 90407706c66f5993fd9a2120f8ff9ab31ac94401 Mon Sep 17 00:00:00 2001 From: Konstantin Kopachev Date: Sat, 9 Jan 2021 15:05:36 -0800 Subject: [PATCH 1/7] only use TIFFReadRGBA* in case of o_jpeg compression --- Tests/test_file_libtiff.py | 4 ---- src/PIL/TiffImagePlugin.py | 9 +++++++++ src/libImaging/TiffDecode.c | 11 ++++++++++- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/Tests/test_file_libtiff.py b/Tests/test_file_libtiff.py index 4d483015f43..3adfc8fed4a 100644 --- a/Tests/test_file_libtiff.py +++ b/Tests/test_file_libtiff.py @@ -814,13 +814,11 @@ def test_strip_cmyk_16l_jpeg(self): with Image.open(infile) as im: assert_image_similar_tofile(im, "Tests/images/pil_sample_cmyk.jpg", 0.5) - @pytest.mark.xfail(is_big_endian(), reason="Fails on big-endian") def test_strip_ycbcr_jpeg_2x2_sampling(self): infile = "Tests/images/tiff_strip_ycbcr_jpeg_2x2_sampling.tif" with Image.open(infile) as im: assert_image_similar_tofile(im, "Tests/images/flower.jpg", 0.5) - @pytest.mark.xfail(is_big_endian(), reason="Fails on big-endian") def test_strip_ycbcr_jpeg_1x1_sampling(self): infile = "Tests/images/tiff_strip_ycbcr_jpeg_1x1_sampling.tif" with Image.open(infile) as im: @@ -831,13 +829,11 @@ def test_tiled_cmyk_jpeg(self): with Image.open(infile) as im: assert_image_similar_tofile(im, "Tests/images/pil_sample_cmyk.jpg", 0.5) - @pytest.mark.xfail(is_big_endian(), reason="Fails on big-endian") def test_tiled_ycbcr_jpeg_1x1_sampling(self): infile = "Tests/images/tiff_tiled_ycbcr_jpeg_1x1_sampling.tif" with Image.open(infile) as im: assert_image_equal_tofile(im, "Tests/images/flower2.jpg") - @pytest.mark.xfail(is_big_endian(), reason="Fails on big-endian") def test_tiled_ycbcr_jpeg_2x2_sampling(self): infile = "Tests/images/tiff_tiled_ycbcr_jpeg_2x2_sampling.tif" with Image.open(infile) as im: diff --git a/src/PIL/TiffImagePlugin.py b/src/PIL/TiffImagePlugin.py index 0b70ce38210..e9a6e49f0cb 100644 --- a/src/PIL/TiffImagePlugin.py +++ b/src/PIL/TiffImagePlugin.py @@ -1324,6 +1324,15 @@ def _setup(self): if ";16L" in rawmode: rawmode = rawmode.replace(";16L", ";16N") + # YCbCr images with new jpeg compression with pixels in one plane + # unpacked straight into RGB values + if ( + photo == 6 + and self._compression == "jpeg" + and self._planar_configuration == 1 + ): + rawmode = "RGB" + # Offset in the tile tuple is 0, we go from 0,0 to # w,h, and we only do this once -- eds a = (rawmode, self._compression, False, self.tag_v2.offset) diff --git a/src/libImaging/TiffDecode.c b/src/libImaging/TiffDecode.c index 55ae32b9b0c..42c782cee09 100644 --- a/src/libImaging/TiffDecode.c +++ b/src/libImaging/TiffDecode.c @@ -401,6 +401,7 @@ ImagingLibTiffDecode( char *mode = "r"; TIFF *tiff; uint16 photometric = 0; // init to not PHOTOMETRIC_YCBCR + uint16 compression; int isYCbCr = 0; uint16 planarconfig = 0; int planes = 1; @@ -502,9 +503,17 @@ ImagingLibTiffDecode( } TIFFGetField(tiff, TIFFTAG_PHOTOMETRIC, &photometric); - isYCbCr = photometric == PHOTOMETRIC_YCBCR; + TIFFGetField(tiff, TIFFTAG_COMPRESSION, &compression); TIFFGetFieldDefaulted(tiff, TIFFTAG_PLANARCONFIG, &planarconfig); + isYCbCr = photometric == PHOTOMETRIC_YCBCR; + + if (isYCbCr && compression == COMPRESSION_JPEG && planarconfig == PLANARCONFIG_CONTIG) { + // If using new JPEG compression, let libjpeg do RGB convertion + TIFFSetField(tiff, TIFFTAG_JPEGCOLORMODE, JPEGCOLORMODE_RGB); + isYCbCr = 0; + } + // YCbCr data is read as RGB by libtiff and we don't need to worry about planar storage in that case // if number of bands is 1, there is no difference with contig case if (planarconfig == PLANARCONFIG_SEPARATE && From 7e5df7ff801ac9d7341b7180cd7a72fed52d1a56 Mon Sep 17 00:00:00 2001 From: Konstantin Kopachev Date: Mon, 11 Jan 2021 22:06:49 -0800 Subject: [PATCH 2/7] Swap pixel values on Big Endian --- Tests/test_file_libtiff.py | 2 -- src/libImaging/TiffDecode.c | 8 ++++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/Tests/test_file_libtiff.py b/Tests/test_file_libtiff.py index 3adfc8fed4a..23f676e61ad 100644 --- a/Tests/test_file_libtiff.py +++ b/Tests/test_file_libtiff.py @@ -16,7 +16,6 @@ assert_image_similar, assert_image_similar_tofile, hopper, - is_big_endian, skip_unless_feature, ) @@ -879,7 +878,6 @@ def test_strip_planar_16bit_RGBa(self): with Image.open("Tests/images/tiff_strip_planar_16bit_RGBa.tiff") as im: assert_image_equal_tofile(im, "Tests/images/tiff_16bit_RGBa_target.png") - @pytest.mark.xfail(is_big_endian(), reason="Fails on big-endian") def test_old_style_jpeg(self): infile = "Tests/images/old-style-jpeg-compression.tif" with Image.open(infile) as im: diff --git a/src/libImaging/TiffDecode.c b/src/libImaging/TiffDecode.c index 42c782cee09..b5b652d43f3 100644 --- a/src/libImaging/TiffDecode.c +++ b/src/libImaging/TiffDecode.c @@ -289,6 +289,10 @@ _decodeStripYCbCr(Imaging im, ImagingCodecState state, TIFF *tiff) { goto decodeycbcr_err; } +#if WORDS_BIGENDIAN + TIFFSwabArrayOfLong((UINT32 *)state->buffer, img.width * rows_to_read); +#endif + TRACE(("Decoded strip for row %d \n", state->y)); // iterate over each row in the strip and stuff data into image @@ -609,6 +613,10 @@ ImagingLibTiffDecode( state->errcode = IMAGING_CODEC_BROKEN; goto decode_err; } + +#if WORDS_BIGENDIAN + TIFFSwabArrayOfLong((UINT32 *)state->buffer, tile_width * tile_length); +#endif } else { if (TIFFReadTile(tiff, (tdata_t)state->buffer, x, y, 0, plane) == -1) { TRACE(("Decode Error, Tile at %dx%d\n", x, y)); From e81e1059aaca5de5c8f97ec5bcad2b99c0d1430c Mon Sep 17 00:00:00 2001 From: Konstantin Kopachev Date: Mon, 11 Jan 2021 23:28:58 -0800 Subject: [PATCH 3/7] unify reading of YCbCr Tiffs --- src/libImaging/TiffDecode.c | 284 +++++++++++++++++------------------- 1 file changed, 132 insertions(+), 152 deletions(-) diff --git a/src/libImaging/TiffDecode.c b/src/libImaging/TiffDecode.c index b5b652d43f3..214f52b871f 100644 --- a/src/libImaging/TiffDecode.c +++ b/src/libImaging/TiffDecode.c @@ -209,24 +209,34 @@ ImagingLibTiffInit(ImagingCodecState state, int fp, uint32 offset) { } int -_decodeStripYCbCr(Imaging im, ImagingCodecState state, TIFF *tiff) { +_decodeYCbCr(Imaging im, ImagingCodecState state, TIFF *tiff) { // To avoid dealing with YCbCr subsampling, let libtiff handle it // Use a TIFFRGBAImage wrapping the tiff image, and let libtiff handle // all of the conversion. Metadata read from the TIFFRGBAImage could // be different from the metadata that the base tiff returns. - INT32 strip_row; + INT32 current_row; UINT8 *new_data; - UINT32 rows_per_strip, row_byte_size, rows_to_read; + UINT32 rows_per_block, row_byte_size, rows_to_read; int ret; TIFFRGBAImage img; char emsg[1024] = ""; - ret = TIFFGetFieldDefaulted(tiff, TIFFTAG_ROWSPERSTRIP, &rows_per_strip); + // Since using TIFFRGBAImage* functions, we can read whole tiff into rastrr in one call + // Let's select smaller block size. Multiplying image width by (tile length OR rows per strip) + // gives us manageable block size in pixels + if (TIFFIsTiled(tiff)) { + ret = TIFFGetFieldDefaulted(tiff, TIFFTAG_TILELENGTH, &rows_per_block); + } + else { + ret = TIFFGetFieldDefaulted(tiff, TIFFTAG_ROWSPERSTRIP, &rows_per_block); + } + if (ret != 1) { - rows_per_strip = state->ysize; + rows_per_block = state->ysize; } - TRACE(("RowsPerStrip: %u \n", rows_per_strip)); + + TRACE(("RowsPerBlock: %u \n", rows_per_block)); if (!(TIFFRGBAImageOK(tiff, emsg) && TIFFRGBAImageBegin(&img, tiff, 0, emsg))) { TRACE(("Decode error, msg: %s", emsg)); @@ -259,14 +269,14 @@ _decodeStripYCbCr(Imaging im, ImagingCodecState state, TIFF *tiff) { row_byte_size = img.width * 4; /* overflow check for realloc */ - if (INT_MAX / row_byte_size < rows_per_strip) { + if (INT_MAX / row_byte_size < rows_per_block) { state->errcode = IMAGING_CODEC_MEMORY; goto decodeycbcr_err; } - state->bytes = rows_per_strip * row_byte_size; + state->bytes = rows_per_block * row_byte_size; - TRACE(("StripSize: %d \n", state->bytes)); + TRACE(("BlockSize: %d \n", state->bytes)); /* realloc to fit whole strip */ /* malloc check above */ @@ -278,9 +288,9 @@ _decodeStripYCbCr(Imaging im, ImagingCodecState state, TIFF *tiff) { state->buffer = new_data; - for (; state->y < state->ysize; state->y += rows_per_strip) { + for (; state->y < state->ysize; state->y += rows_per_block) { img.row_offset = state->y; - rows_to_read = min(rows_per_strip, img.height - state->y); + rows_to_read = min(rows_per_block, img.height - state->y); if (TIFFRGBAImageGet(&img, (UINT32 *)state->buffer, img.width, rows_to_read) == -1) { @@ -296,19 +306,19 @@ _decodeStripYCbCr(Imaging im, ImagingCodecState state, TIFF *tiff) { TRACE(("Decoded strip for row %d \n", state->y)); // iterate over each row in the strip and stuff data into image - for (strip_row = 0; - strip_row < min((INT32)rows_per_strip, state->ysize - state->y); - strip_row++) { - TRACE(("Writing data into line %d ; \n", state->y + strip_row)); + for (current_row = 0; + current_row < min((INT32)rows_per_block, state->ysize - state->y); + current_row++) { + TRACE(("Writing data into line %d ; \n", state->y + current_row)); - // UINT8 * bbb = state->buffer + strip_row * (state->bytes / - // rows_per_strip); TRACE(("chars: %x %x %x %x\n", ((UINT8 *)bbb)[0], + // UINT8 * bbb = state->buffer + current_row * (state->bytes / + // rows_per_block); TRACE(("chars: %x %x %x %x\n", ((UINT8 *)bbb)[0], // ((UINT8 *)bbb)[1], ((UINT8 *)bbb)[2], ((UINT8 *)bbb)[3])); state->shuffle( - (UINT8 *)im->image[state->y + state->yoff + strip_row] + + (UINT8 *)im->image[state->y + state->yoff + current_row] + state->xoff * im->pixelsize, - state->buffer + strip_row * row_byte_size, + state->buffer + current_row * row_byte_size, state->xsize); } } @@ -518,172 +528,142 @@ ImagingLibTiffDecode( isYCbCr = 0; } - // YCbCr data is read as RGB by libtiff and we don't need to worry about planar storage in that case - // if number of bands is 1, there is no difference with contig case - if (planarconfig == PLANARCONFIG_SEPARATE && - im->bands > 1 && - !isYCbCr) { + if (isYCbCr) { + _decodeYCbCr(im, state, tiff); + } + else { + // YCbCr data is read as RGB by libtiff and we don't need to worry about planar storage in that case + // if number of bands is 1, there is no difference with contig case + if (planarconfig == PLANARCONFIG_SEPARATE && + im->bands > 1 && + !isYCbCr) { + + uint16 bits_per_sample = 8; + + TIFFGetFieldDefaulted(tiff, TIFFTAG_BITSPERSAMPLE, &bits_per_sample); + if (bits_per_sample != 8 && bits_per_sample != 16) { + TRACE(("Invalid value for bits per sample: %d\n", bits_per_sample)); + state->errcode = IMAGING_CODEC_BROKEN; + goto decode_err; + } - uint16 bits_per_sample = 8; + planes = im->bands; - TIFFGetFieldDefaulted(tiff, TIFFTAG_BITSPERSAMPLE, &bits_per_sample); - if (bits_per_sample != 8 && bits_per_sample != 16) { - TRACE(("Invalid value for bits per sample: %d\n", bits_per_sample)); - state->errcode = IMAGING_CODEC_BROKEN; - goto decode_err; + // We'll pick appropriate set of unpackers depending on planar_configuration + // It does not matter if data is RGB(A), CMYK or LUV really, + // we just copy it plane by plane + unpackers[0] = ImagingFindUnpacker("RGBA", bits_per_sample == 16 ? "R;16N" : "R", NULL); + unpackers[1] = ImagingFindUnpacker("RGBA", bits_per_sample == 16 ? "G;16N" : "G", NULL); + unpackers[2] = ImagingFindUnpacker("RGBA", bits_per_sample == 16 ? "B;16N" : "B", NULL); + unpackers[3] = ImagingFindUnpacker("RGBA", bits_per_sample == 16 ? "A;16N" : "A", NULL); + } else { + unpackers[0] = state->shuffle; } - planes = im->bands; - - // We'll pick appropriate set of unpackers depending on planar_configuration - // It does not matter if data is RGB(A), CMYK or LUV really, - // we just copy it plane by plane - unpackers[0] = ImagingFindUnpacker("RGBA", bits_per_sample == 16 ? "R;16N" : "R", NULL); - unpackers[1] = ImagingFindUnpacker("RGBA", bits_per_sample == 16 ? "G;16N" : "G", NULL); - unpackers[2] = ImagingFindUnpacker("RGBA", bits_per_sample == 16 ? "B;16N" : "B", NULL); - unpackers[3] = ImagingFindUnpacker("RGBA", bits_per_sample == 16 ? "A;16N" : "A", NULL); - } else { - unpackers[0] = state->shuffle; - } - - if (TIFFIsTiled(tiff)) { - INT32 x, y, tile_y; - UINT32 tile_width, tile_length, current_tile_length, current_line, - current_tile_width, row_byte_size; - UINT8 *new_data; - - TIFFGetField(tiff, TIFFTAG_TILEWIDTH, &tile_width); - TIFFGetField(tiff, TIFFTAG_TILELENGTH, &tile_length); + if (TIFFIsTiled(tiff)) { + INT32 x, y, tile_y; + UINT32 tile_width, tile_length, current_tile_length, current_line, + current_tile_width, row_byte_size; + UINT8 *new_data; - /* overflow check for row_byte_size calculation */ - if ((UINT32)INT_MAX / state->bits < tile_width) { - state->errcode = IMAGING_CODEC_MEMORY; - goto decode_err; - } + TIFFGetField(tiff, TIFFTAG_TILEWIDTH, &tile_width); + TIFFGetField(tiff, TIFFTAG_TILELENGTH, &tile_length); - if (isYCbCr) { - row_byte_size = tile_width * 4; - /* sanity check, we use this value in shuffle below */ - if (im->pixelsize != 4) { - state->errcode = IMAGING_CODEC_BROKEN; + /* overflow check for row_byte_size calculation */ + if ((UINT32)INT_MAX / state->bits < tile_width) { + state->errcode = IMAGING_CODEC_MEMORY; goto decode_err; } - } else { + // We could use TIFFTileSize, but for YCbCr data it returns subsampled data // size row_byte_size = (tile_width * state->bits / planes + 7) / 8; - } - - /* overflow check for realloc */ - if (INT_MAX / row_byte_size < tile_length) { - state->errcode = IMAGING_CODEC_MEMORY; - goto decode_err; - } - state->bytes = row_byte_size * tile_length; + /* overflow check for realloc */ + if (INT_MAX / row_byte_size < tile_length) { + state->errcode = IMAGING_CODEC_MEMORY; + goto decode_err; + } - if (TIFFTileSize(tiff) > state->bytes) { - // If the tile size as expected by LibTiff isn't what we're expecting, - // abort. - state->errcode = IMAGING_CODEC_MEMORY; - goto decode_err; - } + state->bytes = row_byte_size * tile_length; - /* realloc to fit whole tile */ - /* malloc check above */ - new_data = realloc(state->buffer, state->bytes); - if (!new_data) { - state->errcode = IMAGING_CODEC_MEMORY; - goto decode_err; - } + if (TIFFTileSize(tiff) > state->bytes) { + // If the tile size as expected by LibTiff isn't what we're expecting, + // abort. + state->errcode = IMAGING_CODEC_MEMORY; + goto decode_err; + } - state->buffer = new_data; + /* realloc to fit whole tile */ + /* malloc check above */ + new_data = realloc(state->buffer, state->bytes); + if (!new_data) { + state->errcode = IMAGING_CODEC_MEMORY; + goto decode_err; + } - TRACE(("TIFFTileSize: %d\n", state->bytes)); + state->buffer = new_data; - for (y = state->yoff; y < state->ysize; y += tile_length) { - int plane; - for (plane = 0; plane < planes; plane++) { - ImagingShuffler shuffler = unpackers[plane]; - for (x = state->xoff; x < state->xsize; x += tile_width) { - if (isYCbCr) { - /* To avoid dealing with YCbCr subsampling, let libtiff handle it */ - if (!TIFFReadRGBATile(tiff, x, y, (UINT32 *)state->buffer)) { - TRACE(("Decode Error, Tile at %dx%d\n", x, y)); - state->errcode = IMAGING_CODEC_BROKEN; - goto decode_err; - } + TRACE(("TIFFTileSize: %d\n", state->bytes)); -#if WORDS_BIGENDIAN - TIFFSwabArrayOfLong((UINT32 *)state->buffer, tile_width * tile_length); -#endif - } else { + for (y = state->yoff; y < state->ysize; y += tile_length) { + int plane; + for (plane = 0; plane < planes; plane++) { + ImagingShuffler shuffler = unpackers[plane]; + for (x = state->xoff; x < state->xsize; x += tile_width) { if (TIFFReadTile(tiff, (tdata_t)state->buffer, x, y, 0, plane) == -1) { TRACE(("Decode Error, Tile at %dx%d\n", x, y)); state->errcode = IMAGING_CODEC_BROKEN; goto decode_err; } - } - TRACE(("Read tile at %dx%d; \n\n", x, y)); - - current_tile_width = min((INT32) tile_width, state->xsize - x); - current_tile_length = min((INT32) tile_length, state->ysize - y); - // iterate over each line in the tile and stuff data into image - for (tile_y = 0; tile_y < current_tile_length; tile_y++) { - TRACE(("Writing tile data at %dx%d using tile_width: %d; \n", tile_y + y, x, current_tile_width)); - - // UINT8 * bbb = state->buffer + tile_y * row_byte_size; - // TRACE(("chars: %x%x%x%x\n", ((UINT8 *)bbb)[0], ((UINT8 *)bbb)[1], ((UINT8 *)bbb)[2], ((UINT8 *)bbb)[3])); - /* - * For some reason the TIFFReadRGBATile() function - * chooses the lower left corner as the origin. - * Vertically mirror by shuffling the scanlines - * backwards - */ - - if (isYCbCr) { - current_line = tile_length - tile_y - 1; - } else { + TRACE(("Read tile at %dx%d; \n\n", x, y)); + + current_tile_width = min((INT32) tile_width, state->xsize - x); + current_tile_length = min((INT32) tile_length, state->ysize - y); + // iterate over each line in the tile and stuff data into image + for (tile_y = 0; tile_y < current_tile_length; tile_y++) { + TRACE(("Writing tile data at %dx%d using tile_width: %d; \n", tile_y + y, x, current_tile_width)); + + // UINT8 * bbb = state->buffer + tile_y * row_byte_size; + // TRACE(("chars: %x%x%x%x\n", ((UINT8 *)bbb)[0], ((UINT8 *)bbb)[1], ((UINT8 *)bbb)[2], ((UINT8 *)bbb)[3])); + current_line = tile_y; - } - shuffler((UINT8*) im->image[tile_y + y] + x * im->pixelsize, - state->buffer + current_line * row_byte_size, - current_tile_width - ); + shuffler((UINT8*) im->image[tile_y + y] + x * im->pixelsize, + state->buffer + current_line * row_byte_size, + current_tile_width + ); + } } } } } - } else { - if (!isYCbCr) { + else { _decodeStrip(im, state, tiff, planes, unpackers); - } else { - _decodeStripYCbCr(im, state, tiff); } - } - if (!state->errcode) { - // Check if raw mode was RGBa and it was stored on separate planes - // so we have to convert it to RGBA - if (planes > 3 && strcmp(im->mode, "RGBA") == 0) { - uint16 extrasamples; - uint16* sampleinfo; - ImagingShuffler shuffle; - INT32 y; - - TIFFGetFieldDefaulted(tiff, TIFFTAG_EXTRASAMPLES, &extrasamples, &sampleinfo); - - if (extrasamples >= 1 && - (sampleinfo[0] == EXTRASAMPLE_UNSPECIFIED || sampleinfo[0] == EXTRASAMPLE_ASSOCALPHA) - ) { - shuffle = ImagingFindUnpacker("RGBA", "RGBa", NULL); - - for (y = state->yoff; y < state->ysize; y++) { - UINT8* ptr = (UINT8*) im->image[y + state->yoff] + - state->xoff * im->pixelsize; - shuffle(ptr, ptr, state->xsize); + if (!state->errcode) { + // Check if raw mode was RGBa and it was stored on separate planes + // so we have to convert it to RGBA + if (planes > 3 && strcmp(im->mode, "RGBA") == 0) { + uint16 extrasamples; + uint16* sampleinfo; + ImagingShuffler shuffle; + INT32 y; + + TIFFGetFieldDefaulted(tiff, TIFFTAG_EXTRASAMPLES, &extrasamples, &sampleinfo); + + if (extrasamples >= 1 && + (sampleinfo[0] == EXTRASAMPLE_UNSPECIFIED || sampleinfo[0] == EXTRASAMPLE_ASSOCALPHA) + ) { + shuffle = ImagingFindUnpacker("RGBA", "RGBa", NULL); + + for (y = state->yoff; y < state->ysize; y++) { + UINT8* ptr = (UINT8*) im->image[y + state->yoff] + + state->xoff * im->pixelsize; + shuffle(ptr, ptr, state->xsize); + } } } } From 727760093356630a1880952d3694dd1b26d5be73 Mon Sep 17 00:00:00 2001 From: Konstantin Kopachev Date: Wed, 13 Jan 2021 18:33:49 -0800 Subject: [PATCH 4/7] Refactor into smaller functions --- src/libImaging/TiffDecode.c | 251 +++++++++++++++++++----------------- 1 file changed, 133 insertions(+), 118 deletions(-) diff --git a/src/libImaging/TiffDecode.c b/src/libImaging/TiffDecode.c index 214f52b871f..011b796b475 100644 --- a/src/libImaging/TiffDecode.c +++ b/src/libImaging/TiffDecode.c @@ -209,8 +209,37 @@ ImagingLibTiffInit(ImagingCodecState state, int fp, uint32 offset) { } int -_decodeYCbCr(Imaging im, ImagingCodecState state, TIFF *tiff) { - // To avoid dealing with YCbCr subsampling, let libtiff handle it +_pickUnpackers(Imaging im, ImagingCodecState state, TIFF *tiff, uint16 planarconfig, ImagingShuffler *unpackers) { + // if number of bands is 1, there is no difference with contig case + if (planarconfig == PLANARCONFIG_SEPARATE && im->bands > 1) { + uint16 bits_per_sample = 8; + + TIFFGetFieldDefaulted(tiff, TIFFTAG_BITSPERSAMPLE, &bits_per_sample); + if (bits_per_sample != 8 && bits_per_sample != 16) { + TRACE(("Invalid value for bits per sample: %d\n", bits_per_sample)); + state->errcode = IMAGING_CODEC_BROKEN; + return -1; + } + + // We'll pick appropriate set of unpackers depending on planar_configuration + // It does not matter if data is RGB(A), CMYK or LUV really, + // we just copy it plane by plane + unpackers[0] = ImagingFindUnpacker("RGBA", bits_per_sample == 16 ? "R;16N" : "R", NULL); + unpackers[1] = ImagingFindUnpacker("RGBA", bits_per_sample == 16 ? "G;16N" : "G", NULL); + unpackers[2] = ImagingFindUnpacker("RGBA", bits_per_sample == 16 ? "B;16N" : "B", NULL); + unpackers[3] = ImagingFindUnpacker("RGBA", bits_per_sample == 16 ? "A;16N" : "A", NULL); + + return im->bands; + } else { + unpackers[0] = state->shuffle; + + return 1; + } +} + +int +_decodeAsRGBA(Imaging im, ImagingCodecState state, TIFF *tiff) { + // To avoid dealing with YCbCr subsampling and other complications, let libtiff handle it // Use a TIFFRGBAImage wrapping the tiff image, and let libtiff handle // all of the conversion. Metadata read from the TIFFRGBAImage could // be different from the metadata that the base tiff returns. @@ -256,13 +285,13 @@ _decodeYCbCr(Imaging im, ImagingCodecState state, TIFF *tiff) { state->ysize, img.height)); state->errcode = IMAGING_CODEC_BROKEN; - goto decodeycbcr_err; + goto decodergba_err; } /* overflow check for row byte size */ if (INT_MAX / 4 < img.width) { state->errcode = IMAGING_CODEC_MEMORY; - goto decodeycbcr_err; + goto decodergba_err; } // TiffRGBAImages are 32bits/pixel. @@ -271,7 +300,7 @@ _decodeYCbCr(Imaging im, ImagingCodecState state, TIFF *tiff) { /* overflow check for realloc */ if (INT_MAX / row_byte_size < rows_per_block) { state->errcode = IMAGING_CODEC_MEMORY; - goto decodeycbcr_err; + goto decodergba_err; } state->bytes = rows_per_block * row_byte_size; @@ -283,7 +312,7 @@ _decodeYCbCr(Imaging im, ImagingCodecState state, TIFF *tiff) { new_data = realloc(state->buffer, state->bytes); if (!new_data) { state->errcode = IMAGING_CODEC_MEMORY; - goto decodeycbcr_err; + goto decodergba_err; } state->buffer = new_data; @@ -296,7 +325,7 @@ _decodeYCbCr(Imaging im, ImagingCodecState state, TIFF *tiff) { -1) { TRACE(("Decode Error, y: %d\n", state->y)); state->errcode = IMAGING_CODEC_BROKEN; - goto decodeycbcr_err; + goto decodergba_err; } #if WORDS_BIGENDIAN @@ -323,7 +352,7 @@ _decodeYCbCr(Imaging im, ImagingCodecState state, TIFF *tiff) { } } -decodeycbcr_err: +decodergba_err: TIFFRGBAImageEnd(&img); if (state->errcode != 0) { return -1; @@ -331,6 +360,89 @@ _decodeYCbCr(Imaging im, ImagingCodecState state, TIFF *tiff) { return 0; } +int +_decodeTile(Imaging im, ImagingCodecState state, TIFF *tiff, int planes, ImagingShuffler *unpackers) { + INT32 x, y, tile_y; + UINT32 tile_width, tile_length, current_tile_length, current_line, + current_tile_width, row_byte_size; + UINT8 *new_data; + + TIFFGetField(tiff, TIFFTAG_TILEWIDTH, &tile_width); + TIFFGetField(tiff, TIFFTAG_TILELENGTH, &tile_length); + + /* overflow check for row_byte_size calculation */ + if ((UINT32)INT_MAX / state->bits < tile_width) { + state->errcode = IMAGING_CODEC_MEMORY; + return -1; + } + + // We could use TIFFTileSize, but for YCbCr data it returns subsampled data + // size + row_byte_size = (tile_width * state->bits / planes + 7) / 8; + + /* overflow check for realloc */ + if (INT_MAX / row_byte_size < tile_length) { + state->errcode = IMAGING_CODEC_MEMORY; + return -1; + } + + state->bytes = row_byte_size * tile_length; + + if (TIFFTileSize(tiff) > state->bytes) { + // If the tile size as expected by LibTiff isn't what we're expecting, + // abort. + state->errcode = IMAGING_CODEC_MEMORY; + return -1; + } + + /* realloc to fit whole tile */ + /* malloc check above */ + new_data = realloc(state->buffer, state->bytes); + if (!new_data) { + state->errcode = IMAGING_CODEC_MEMORY; + return -1; + } + + state->buffer = new_data; + + TRACE(("TIFFTileSize: %d\n", state->bytes)); + + for (y = state->yoff; y < state->ysize; y += tile_length) { + int plane; + for (plane = 0; plane < planes; plane++) { + ImagingShuffler shuffler = unpackers[plane]; + for (x = state->xoff; x < state->xsize; x += tile_width) { + if (TIFFReadTile(tiff, (tdata_t)state->buffer, x, y, 0, plane) == -1) { + TRACE(("Decode Error, Tile at %dx%d\n", x, y)); + state->errcode = IMAGING_CODEC_BROKEN; + return -1; + } + + TRACE(("Read tile at %dx%d; \n\n", x, y)); + + current_tile_width = min((INT32) tile_width, state->xsize - x); + current_tile_length = min((INT32) tile_length, state->ysize - y); + // iterate over each line in the tile and stuff data into image + for (tile_y = 0; tile_y < current_tile_length; tile_y++) { + TRACE(("Writing tile data at %dx%d using tile_width: %d; \n", tile_y + y, x, current_tile_width)); + + // UINT8 * bbb = state->buffer + tile_y * row_byte_size; + // TRACE(("chars: %x%x%x%x\n", ((UINT8 *)bbb)[0], ((UINT8 *)bbb)[1], ((UINT8 *)bbb)[2], ((UINT8 *)bbb)[3])); + + current_line = tile_y; + + shuffler((UINT8*) im->image[tile_y + y] + x * im->pixelsize, + state->buffer + current_line * row_byte_size, + current_tile_width + ); + } + } + } + } + + return 0; +} + int _decodeStrip(Imaging im, ImagingCodecState state, TIFF *tiff, int planes, ImagingShuffler *unpackers) { INT32 strip_row = 0; @@ -416,7 +528,7 @@ ImagingLibTiffDecode( TIFF *tiff; uint16 photometric = 0; // init to not PHOTOMETRIC_YCBCR uint16 compression; - int isYCbCr = 0; + int readAsRGBA = 0; uint16 planarconfig = 0; int planes = 1; ImagingShuffler unpackers[4]; @@ -520,124 +632,27 @@ ImagingLibTiffDecode( TIFFGetField(tiff, TIFFTAG_COMPRESSION, &compression); TIFFGetFieldDefaulted(tiff, TIFFTAG_PLANARCONFIG, &planarconfig); - isYCbCr = photometric == PHOTOMETRIC_YCBCR; + // Dealing with YCbCr images is complicated in case if subsampling + // Let LibTiff read them as RGBA + readAsRGBA = photometric == PHOTOMETRIC_YCBCR; - if (isYCbCr && compression == COMPRESSION_JPEG && planarconfig == PLANARCONFIG_CONTIG) { - // If using new JPEG compression, let libjpeg do RGB convertion + if (readAsRGBA && compression == COMPRESSION_JPEG && planarconfig == PLANARCONFIG_CONTIG) { + // If using new JPEG compression, let libjpeg do RGB convertion for performance reasons TIFFSetField(tiff, TIFFTAG_JPEGCOLORMODE, JPEGCOLORMODE_RGB); - isYCbCr = 0; + readAsRGBA = 0; } - if (isYCbCr) { - _decodeYCbCr(im, state, tiff); + if (readAsRGBA) { + _decodeAsRGBA(im, state, tiff); } else { - // YCbCr data is read as RGB by libtiff and we don't need to worry about planar storage in that case - // if number of bands is 1, there is no difference with contig case - if (planarconfig == PLANARCONFIG_SEPARATE && - im->bands > 1 && - !isYCbCr) { - - uint16 bits_per_sample = 8; - - TIFFGetFieldDefaulted(tiff, TIFFTAG_BITSPERSAMPLE, &bits_per_sample); - if (bits_per_sample != 8 && bits_per_sample != 16) { - TRACE(("Invalid value for bits per sample: %d\n", bits_per_sample)); - state->errcode = IMAGING_CODEC_BROKEN; - goto decode_err; - } - - planes = im->bands; - - // We'll pick appropriate set of unpackers depending on planar_configuration - // It does not matter if data is RGB(A), CMYK or LUV really, - // we just copy it plane by plane - unpackers[0] = ImagingFindUnpacker("RGBA", bits_per_sample == 16 ? "R;16N" : "R", NULL); - unpackers[1] = ImagingFindUnpacker("RGBA", bits_per_sample == 16 ? "G;16N" : "G", NULL); - unpackers[2] = ImagingFindUnpacker("RGBA", bits_per_sample == 16 ? "B;16N" : "B", NULL); - unpackers[3] = ImagingFindUnpacker("RGBA", bits_per_sample == 16 ? "A;16N" : "A", NULL); - } else { - unpackers[0] = state->shuffle; + planes = _pickUnpackers(im, state, tiff, planarconfig, unpackers); + if (planes <= 0) { + goto decode_err; } if (TIFFIsTiled(tiff)) { - INT32 x, y, tile_y; - UINT32 tile_width, tile_length, current_tile_length, current_line, - current_tile_width, row_byte_size; - UINT8 *new_data; - - TIFFGetField(tiff, TIFFTAG_TILEWIDTH, &tile_width); - TIFFGetField(tiff, TIFFTAG_TILELENGTH, &tile_length); - - /* overflow check for row_byte_size calculation */ - if ((UINT32)INT_MAX / state->bits < tile_width) { - state->errcode = IMAGING_CODEC_MEMORY; - goto decode_err; - } - - // We could use TIFFTileSize, but for YCbCr data it returns subsampled data - // size - row_byte_size = (tile_width * state->bits / planes + 7) / 8; - - /* overflow check for realloc */ - if (INT_MAX / row_byte_size < tile_length) { - state->errcode = IMAGING_CODEC_MEMORY; - goto decode_err; - } - - state->bytes = row_byte_size * tile_length; - - if (TIFFTileSize(tiff) > state->bytes) { - // If the tile size as expected by LibTiff isn't what we're expecting, - // abort. - state->errcode = IMAGING_CODEC_MEMORY; - goto decode_err; - } - - /* realloc to fit whole tile */ - /* malloc check above */ - new_data = realloc(state->buffer, state->bytes); - if (!new_data) { - state->errcode = IMAGING_CODEC_MEMORY; - goto decode_err; - } - - state->buffer = new_data; - - TRACE(("TIFFTileSize: %d\n", state->bytes)); - - for (y = state->yoff; y < state->ysize; y += tile_length) { - int plane; - for (plane = 0; plane < planes; plane++) { - ImagingShuffler shuffler = unpackers[plane]; - for (x = state->xoff; x < state->xsize; x += tile_width) { - if (TIFFReadTile(tiff, (tdata_t)state->buffer, x, y, 0, plane) == -1) { - TRACE(("Decode Error, Tile at %dx%d\n", x, y)); - state->errcode = IMAGING_CODEC_BROKEN; - goto decode_err; - } - - TRACE(("Read tile at %dx%d; \n\n", x, y)); - - current_tile_width = min((INT32) tile_width, state->xsize - x); - current_tile_length = min((INT32) tile_length, state->ysize - y); - // iterate over each line in the tile and stuff data into image - for (tile_y = 0; tile_y < current_tile_length; tile_y++) { - TRACE(("Writing tile data at %dx%d using tile_width: %d; \n", tile_y + y, x, current_tile_width)); - - // UINT8 * bbb = state->buffer + tile_y * row_byte_size; - // TRACE(("chars: %x%x%x%x\n", ((UINT8 *)bbb)[0], ((UINT8 *)bbb)[1], ((UINT8 *)bbb)[2], ((UINT8 *)bbb)[3])); - - current_line = tile_y; - - shuffler((UINT8*) im->image[tile_y + y] + x * im->pixelsize, - state->buffer + current_line * row_byte_size, - current_tile_width - ); - } - } - } - } + _decodeTile(im, state, tiff, planes, unpackers); } else { _decodeStrip(im, state, tiff, planes, unpackers); From 5b5ed7a36f211204a16c1056bdb8c7f4ef7b8839 Mon Sep 17 00:00:00 2001 From: Konstantin Kopachev Date: Mon, 25 Jan 2021 20:29:04 -0800 Subject: [PATCH 5/7] Check for dimensions and sizes to fit into int --- src/libImaging/TiffDecode.c | 80 +++++++++++++++++++------------------ 1 file changed, 42 insertions(+), 38 deletions(-) diff --git a/src/libImaging/TiffDecode.c b/src/libImaging/TiffDecode.c index 011b796b475..6cd37c2d92a 100644 --- a/src/libImaging/TiffDecode.c +++ b/src/libImaging/TiffDecode.c @@ -362,39 +362,35 @@ _decodeAsRGBA(Imaging im, ImagingCodecState state, TIFF *tiff) { int _decodeTile(Imaging im, ImagingCodecState state, TIFF *tiff, int planes, ImagingShuffler *unpackers) { - INT32 x, y, tile_y; - UINT32 tile_width, tile_length, current_tile_length, current_line, - current_tile_width, row_byte_size; + INT32 x, y, tile_y, current_tile_length, current_tile_width; + UINT32 tile_width, tile_length; + tsize_t tile_bytes_size, row_byte_size; UINT8 *new_data; - TIFFGetField(tiff, TIFFTAG_TILEWIDTH, &tile_width); - TIFFGetField(tiff, TIFFTAG_TILELENGTH, &tile_length); + tile_bytes_size = TIFFTileSize(tiff); - /* overflow check for row_byte_size calculation */ - if ((UINT32)INT_MAX / state->bits < tile_width) { - state->errcode = IMAGING_CODEC_MEMORY; + if (tile_bytes_size == 0) { + TRACE(("Decode Error, Can not calculate TileSize\n")); + state->errcode = IMAGING_CODEC_BROKEN; return -1; } - // We could use TIFFTileSize, but for YCbCr data it returns subsampled data - // size - row_byte_size = (tile_width * state->bits / planes + 7) / 8; + row_byte_size = TIFFTileRowSize(tiff); - /* overflow check for realloc */ - if (INT_MAX / row_byte_size < tile_length) { - state->errcode = IMAGING_CODEC_MEMORY; + if (row_byte_size == 0 || row_byte_size > tile_bytes_size) { + TRACE(("Decode Error, Can not calculate TileRowSize\n")); + state->errcode = IMAGING_CODEC_BROKEN; return -1; } - state->bytes = row_byte_size * tile_length; - - if (TIFFTileSize(tiff) > state->bytes) { - // If the tile size as expected by LibTiff isn't what we're expecting, - // abort. + /* overflow check for realloc */ + if (tile_bytes_size > INT_MAX - 1) { state->errcode = IMAGING_CODEC_MEMORY; return -1; } + state->bytes = tile_bytes_size; + /* realloc to fit whole tile */ /* malloc check above */ new_data = realloc(state->buffer, state->bytes); @@ -402,9 +398,17 @@ _decodeTile(Imaging im, ImagingCodecState state, TIFF *tiff, int planes, Imaging state->errcode = IMAGING_CODEC_MEMORY; return -1; } - state->buffer = new_data; + TIFFGetField(tiff, TIFFTAG_TILEWIDTH, &tile_width); + TIFFGetField(tiff, TIFFTAG_TILELENGTH, &tile_length); + + if (tile_width > INT_MAX || tile_length > INT_MAX) { + // state->x and state->y are ints + state->errcode = IMAGING_CODEC_MEMORY; + return -1; + } + TRACE(("TIFFTileSize: %d\n", state->bytes)); for (y = state->yoff; y < state->ysize; y += tile_length) { @@ -429,10 +433,8 @@ _decodeTile(Imaging im, ImagingCodecState state, TIFF *tiff, int planes, Imaging // UINT8 * bbb = state->buffer + tile_y * row_byte_size; // TRACE(("chars: %x%x%x%x\n", ((UINT8 *)bbb)[0], ((UINT8 *)bbb)[1], ((UINT8 *)bbb)[2], ((UINT8 *)bbb)[3])); - current_line = tile_y; - shuffler((UINT8*) im->image[tile_y + y] + x * im->pixelsize, - state->buffer + current_line * row_byte_size, + state->buffer + tile_y * row_byte_size, current_tile_width ); } @@ -447,38 +449,40 @@ int _decodeStrip(Imaging im, ImagingCodecState state, TIFF *tiff, int planes, ImagingShuffler *unpackers) { INT32 strip_row = 0; UINT8 *new_data; - UINT32 rows_per_strip, row_byte_size; + UINT32 rows_per_strip; int ret; + tsize_t strip_size, row_byte_size; ret = TIFFGetField(tiff, TIFFTAG_ROWSPERSTRIP, &rows_per_strip); - if (ret != 1) { + if (ret != 1 || rows_per_strip==(UINT32)(-1)) { rows_per_strip = state->ysize; } - TRACE(("RowsPerStrip: %u \n", rows_per_strip)); - - // We could use TIFFStripSize, but for YCbCr data it returns subsampled data size - row_byte_size = (state->xsize * state->bits / planes + 7) / 8; - /* overflow check for realloc */ - if (INT_MAX / row_byte_size < rows_per_strip) { + if (rows_per_strip > INT_MAX) { state->errcode = IMAGING_CODEC_MEMORY; return -1; } - state->bytes = rows_per_strip * row_byte_size; + TRACE(("RowsPerStrip: %u\n", rows_per_strip)); + + strip_size = TIFFStripSize(tiff); + if (strip_size > INT_MAX - 1) { + state->errcode = IMAGING_CODEC_MEMORY; + return -1; + } + state->bytes = strip_size; TRACE(("StripSize: %d \n", state->bytes)); - if (TIFFStripSize(tiff) > state->bytes) { - // If the strip size as expected by LibTiff isn't what we're expecting, abort. - // man: TIFFStripSize returns the equivalent size for a strip of data as it - // would be returned in a - // call to TIFFReadEncodedStrip ... + row_byte_size = TIFFScanlineSize(tiff); - state->errcode = IMAGING_CODEC_MEMORY; + if (row_byte_size == 0 || row_byte_size > strip_size) { + state->errcode = IMAGING_CODEC_BROKEN; return -1; } + TRACE(("RowsByteSize: %u \n", row_byte_size)); + /* realloc to fit whole strip */ /* malloc check above */ new_data = realloc(state->buffer, state->bytes); From 78c273924a9187d31784212706bcefbf7e2f8038 Mon Sep 17 00:00:00 2001 From: Konstantin Kopachev Date: Mon, 25 Jan 2021 21:45:57 -0800 Subject: [PATCH 6/7] Add sanity check for memory overruns --- src/libImaging/TiffDecode.c | 39 ++++++++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/src/libImaging/TiffDecode.c b/src/libImaging/TiffDecode.c index 6cd37c2d92a..266bcaeff25 100644 --- a/src/libImaging/TiffDecode.c +++ b/src/libImaging/TiffDecode.c @@ -389,17 +389,6 @@ _decodeTile(Imaging im, ImagingCodecState state, TIFF *tiff, int planes, Imaging return -1; } - state->bytes = tile_bytes_size; - - /* realloc to fit whole tile */ - /* malloc check above */ - new_data = realloc(state->buffer, state->bytes); - if (!new_data) { - state->errcode = IMAGING_CODEC_MEMORY; - return -1; - } - state->buffer = new_data; - TIFFGetField(tiff, TIFFTAG_TILEWIDTH, &tile_width); TIFFGetField(tiff, TIFFTAG_TILELENGTH, &tile_length); @@ -409,8 +398,27 @@ _decodeTile(Imaging im, ImagingCodecState state, TIFF *tiff, int planes, Imaging return -1; } + if (tile_bytes_size > ((tile_length * state->bits / planes + 7) / 8) * tile_width) { + // If the tile size as expected by LibTiff isn't what we're expecting, abort. + // man: TIFFTileSize returns the equivalent size for a tile of data as it would be returned in a + // call to TIFFReadTile ... + state->errcode = IMAGING_CODEC_BROKEN; + return -1; + } + + state->bytes = tile_bytes_size; + TRACE(("TIFFTileSize: %d\n", state->bytes)); + /* realloc to fit whole tile */ + /* malloc check above */ + new_data = realloc(state->buffer, state->bytes); + if (!new_data) { + state->errcode = IMAGING_CODEC_MEMORY; + return -1; + } + state->buffer = new_data; + for (y = state->yoff; y < state->ysize; y += tile_length) { int plane; for (plane = 0; plane < planes; plane++) { @@ -470,6 +478,15 @@ _decodeStrip(Imaging im, ImagingCodecState state, TIFF *tiff, int planes, Imagin state->errcode = IMAGING_CODEC_MEMORY; return -1; } + + if (strip_size > ((state->xsize * state->bits / planes + 7) / 8) * rows_per_strip) { + // If the strip size as expected by LibTiff isn't what we're expecting, abort. + // man: TIFFStripSize returns the equivalent size for a strip of data as it would be returned in a + // call to TIFFReadEncodedStrip ... + state->errcode = IMAGING_CODEC_BROKEN; + return -1; + } + state->bytes = strip_size; TRACE(("StripSize: %d \n", state->bytes)); From de9d2179ec13ad9bfb55c77c17e051229305bf61 Mon Sep 17 00:00:00 2001 From: Konstantin Kopachev Date: Mon, 8 Mar 2021 20:20:29 -0800 Subject: [PATCH 7/7] Stop guessing strip size and pass expected size --- src/libImaging/TiffDecode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libImaging/TiffDecode.c b/src/libImaging/TiffDecode.c index 266bcaeff25..da8bd9abd36 100644 --- a/src/libImaging/TiffDecode.c +++ b/src/libImaging/TiffDecode.c @@ -514,7 +514,7 @@ _decodeStrip(Imaging im, ImagingCodecState state, TIFF *tiff, int planes, Imagin int plane; for (plane = 0; plane < planes; plane++) { ImagingShuffler shuffler = unpackers[plane]; - if (TIFFReadEncodedStrip(tiff, TIFFComputeStrip(tiff, state->y, plane), (tdata_t)state->buffer, -1) == -1) { + if (TIFFReadEncodedStrip(tiff, TIFFComputeStrip(tiff, state->y, plane), (tdata_t)state->buffer, strip_size) == -1) { TRACE(("Decode Error, strip %d\n", TIFFComputeStrip(tiff, state->y, 0))); state->errcode = IMAGING_CODEC_BROKEN; return -1;