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 9893d65a63f2538e78b7f34c6a5f2d82ae50f6e1..d4c7a8716d7a6dae7876d3b07030b0a0f8a79fde 100644 (file)
--- a/src/lib/j2k_image_proxy.cc
+++ b/src/lib/j2k_image_proxy.cc
@@ -138,7 +138,13 @@ J2KImageProxy::prepare (optional<dcp::Size> target_size) const
 
        shared_ptr<dcp::OpenJPEGImage> decompressed = dcp::decompress_j2k (const_cast<uint8_t*> (_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 1332f1c52ae4540f544e19c1f4859da046e38caf..8378207cfd42b0971de89cb60ab5a78bff13e524 100644 (file)
--- 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> 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> 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)