From: Carl Hetherington Date: Mon, 4 Nov 2019 11:04:30 +0000 (+0100) Subject: Fix out-of-bounds read when cropping JPEG2000 images (#1654). X-Git-Tag: v2.15.29~3 X-Git-Url: https://main.carlh.net/gitweb/?p=dcpomatic.git;a=commitdiff_plain;h=25d968fdcf1abada4bd7bbcb8c72eeebda73b134 Fix out-of-bounds read when cropping JPEG2000 images (#1654). --- diff --git a/src/lib/j2k_image_proxy.cc b/src/lib/j2k_image_proxy.cc index 9893d65a6..d4c7a8716 100644 --- a/src/lib/j2k_image_proxy.cc +++ b/src/lib/j2k_image_proxy.cc @@ -138,7 +138,13 @@ J2KImageProxy::prepare (optional target_size) const shared_ptr decompressed = dcp::decompress_j2k (const_cast (_data.data().get()), _data.size (), reduce); - _image.reset (new Image (_pixel_format, decompressed->size(), true)); + /* When scaling JPEG2000 images (using AV_PIX_FMT_XYZ12LE) ffmpeg will call xyz12ToRgb48 which reads data + from the whole of the image stride. If we are cropping, Image::crop_scale_window munges the + start addresses of each image row (to do the crop) but keeps the stride the same. This means + that under crop we will read over the end of the image by the amount of the crop. To allow this + to happen without invalid memory access we need to overallocate by one whole stride's worth of pixels. + */ + _image.reset (new Image (_pixel_format, decompressed->size(), true, decompressed->size().width)); int const shift = 16 - decompressed->precision (0); diff --git a/test/image_test.cc b/test/image_test.cc index 1332f1c52..8378207cf 100644 --- a/test/image_test.cc +++ b/test/image_test.cc @@ -267,12 +267,13 @@ BOOST_AUTO_TEST_CASE (crop_scale_window_test) check_image("test/data/crop_scale_window_test.png", "build/test/crop_scale_window_test.png"); } -/** Special case of Image::crop_scale_window which triggered some valgrind warnings */ +/** Special cases of Image::crop_scale_window which triggered some valgrind warnings */ BOOST_AUTO_TEST_CASE (crop_scale_window_test2) { - shared_ptr image (new Image(AV_PIX_FMT_XYZ12LE, dcp::Size(2048, 858), true)); + /* This 2048 does the same as J2KImageProxy does when it makes an image */ + shared_ptr image (new Image(AV_PIX_FMT_XYZ12LE, dcp::Size(2048, 858), true, 2048)); image->crop_scale_window (Crop(279, 0, 0, 0), dcp::Size(1069, 448), dcp::Size(1069, 578), dcp::YUV_TO_RGB_REC709, VIDEO_RANGE_FULL, AV_PIX_FMT_RGB24, false, false); - + image->crop_scale_window (Crop(2048, 0, 0, 0), dcp::Size(1069, 448), dcp::Size(1069, 578), dcp::YUV_TO_RGB_REC709, VIDEO_RANGE_FULL, AV_PIX_FMT_RGB24, false, false); } BOOST_AUTO_TEST_CASE (as_png_test)