From d3dc7c3cc8c85f65a4ee5620d104880d6b8754bf Mon Sep 17 00:00:00 2001 From: Wan-Teh Chang Date: Thu, 11 Apr 2024 10:24:11 -0700 Subject: Avoid integer overflows in arithmetic operations A port of the libaom CL https://aomedia-review.googlesource.com/c/aom/+/188823. Impose maximum values on the input parameters so that we can perform arithmetic operations without worrying about overflows. Also change the VpxImageTest.VpxImgAllocHugeWidth test to write to the first and last samples in the first row of the Y plane, so that the test will crash if there is unsigned integer overflow in the calculation of stride_in_bytes. Bug: chromium:332382766 Change-Id: I54cec6c9e26377abaa8a991042ba277ff70afdf3 (cherry picked from commit 06af417e795e6a9b9309406ba399fb109def89e6) --- test/vpx_image_test.cc | 19 +++++++++++++++++++ vpx/src/vpx_image.c | 11 +++++++++++ vpx/vpx_image.h | 16 +++++++++++----- 3 files changed, 41 insertions(+), 5 deletions(-) diff --git a/test/vpx_image_test.cc b/test/vpx_image_test.cc index 74515df70..eb6e35474 100644 --- a/test/vpx_image_test.cc +++ b/test/vpx_image_test.cc @@ -8,6 +8,8 @@ * be found in the AUTHORS file in the root of the source tree. */ +#include + #include "vpx/vpx_image.h" #include "third_party/googletest/src/include/gtest/gtest.h" @@ -54,6 +56,10 @@ TEST(VpxImageTest, VpxImgAllocHugeWidth) { image = vpx_img_alloc(nullptr, VPX_IMG_FMT_I420, 0x80000000, 1, 1); ASSERT_EQ(image, nullptr); + // The aligned width (UINT_MAX + 1) would overflow unsigned int. + image = vpx_img_alloc(nullptr, VPX_IMG_FMT_I420, UINT_MAX, 1, 1); + ASSERT_EQ(image, nullptr); + image = vpx_img_alloc(nullptr, VPX_IMG_FMT_I420, 0x7ffffffe, 1, 1); if (image) { vpx_img_free(image); @@ -74,8 +80,21 @@ TEST(VpxImageTest, VpxImgAllocHugeWidth) { vpx_img_free(image); } + image = vpx_img_alloc(nullptr, VPX_IMG_FMT_I42016, 65536, 2, 1); + if (image) { + uint16_t *y_plane = + reinterpret_cast(image->planes[VPX_PLANE_Y]); + y_plane[0] = 0; + y_plane[image->d_w - 1] = 0; + vpx_img_free(image); + } + image = vpx_img_alloc(nullptr, VPX_IMG_FMT_I42016, 285245883, 2, 1); if (image) { + uint16_t *y_plane = + reinterpret_cast(image->planes[VPX_PLANE_Y]); + y_plane[0] = 0; + y_plane[image->d_w - 1] = 0; vpx_img_free(image); } } diff --git a/vpx/src/vpx_image.c b/vpx/src/vpx_image.c index ab3c2b231..373b80b60 100644 --- a/vpx/src/vpx_image.c +++ b/vpx/src/vpx_image.c @@ -8,6 +8,7 @@ * be found in the AUTHORS file in the root of the source tree. */ +#include #include #include #include @@ -28,6 +29,14 @@ static vpx_image_t *img_alloc_helper(vpx_image_t *img, vpx_img_fmt_t fmt, if (img != NULL) memset(img, 0, sizeof(vpx_image_t)); + /* Impose maximum values on input parameters so that this function can + * perform arithmetic operations without worrying about overflows. + */ + if (d_w > 0x08000000 || d_h > 0x08000000 || buf_align > 65536 || + stride_align > 65536) { + goto fail; + } + /* Treat align==0 like align==1 */ if (!buf_align) buf_align = 1; @@ -103,8 +112,10 @@ static vpx_image_t *img_alloc_helper(vpx_image_t *img, vpx_img_fmt_t fmt, /* Calculate storage sizes given the chroma subsampling */ align = (1 << xcs) - 1; w = (d_w + align) & ~align; + assert(d_w <= w); align = (1 << ycs) - 1; h = (d_h + align) & ~align; + assert(d_h <= h); s = (fmt & VPX_IMG_FMT_PLANAR) ? w : (uint64_t)bps * w / 8; s = (s + stride_align - 1) & ~((uint64_t)stride_align - 1); diff --git a/vpx/vpx_image.h b/vpx/vpx_image.h index 1adc9b9d9..2c30a8993 100644 --- a/vpx/vpx_image.h +++ b/vpx/vpx_image.h @@ -132,10 +132,13 @@ typedef struct vpx_image_rect { * is NULL, the storage for the descriptor will be * allocated on the heap. * \param[in] fmt Format for the image - * \param[in] d_w Width of the image - * \param[in] d_h Height of the image + * \param[in] d_w Width of the image. Must not exceed 0x08000000 + * (2^27). + * \param[in] d_h Height of the image. Must not exceed 0x08000000 + * (2^27). * \param[in] align Alignment, in bytes, of the image buffer and - * each row in the image(stride). + * each row in the image (stride). Must not exceed + * 65536. * * \return Returns a pointer to the initialized image descriptor. If the img * parameter is non-null, the value of the img parameter will be @@ -155,9 +158,12 @@ vpx_image_t *vpx_img_alloc(vpx_image_t *img, vpx_img_fmt_t fmt, * parameter is NULL, the storage for the descriptor * will be allocated on the heap. * \param[in] fmt Format for the image - * \param[in] d_w Width of the image - * \param[in] d_h Height of the image + * \param[in] d_w Width of the image. Must not exceed 0x08000000 + * (2^27). + * \param[in] d_h Height of the image. Must not exceed 0x08000000 + * (2^27). * \param[in] stride_align Alignment, in bytes, of each row in the image. + * Must not exceed 65536. * \param[in] img_data Storage to use for the image * * \return Returns a pointer to the initialized image descriptor. If the img -- 2.30.2