From 1a13e3bab13f8067fdd11d97caa9fa2360e31a1c Mon Sep 17 00:00:00 2001 From: Wan-Teh Chang Date: Wed, 10 Apr 2024 17:01:10 -0700 Subject: Fix integer overflows in calc of stride_in_bytes A port of the libaom CL https://aomedia-review.googlesource.com/c/aom/+/188761. Fix unsigned integer overflows in the calculation of stride_in_bytes in img_alloc_helper() when d_w is huge. Change the type of stride_in_bytes from unsigned int to int because it will be assigned to img->stride[VPX_PLANE_Y], which is of the int type. Test: . ../libvpx/tools/set_analyzer_env.sh integer ../libvpx/configure --enable-debug --disable-optimizations make -j ./test_libvpx --gtest_filter=VpxImageTest.VpxImgAllocHugeWidth Bug: chromium:332382766 Change-Id: I3b39d78f61c7255e10cbf72ba2f4975425a05a82 (cherry picked from commit 2e32276277c0b1739707c5e861c96cf78794f1a0) --- test/vpx_image_test.cc | 36 ++++++++++++++++++++++++++++++++++++ vpx/src/vpx_image.c | 31 +++++++++++++++++++------------ 2 files changed, 55 insertions(+), 12 deletions(-) diff --git a/test/vpx_image_test.cc b/test/vpx_image_test.cc index c93843c1e..74515df70 100644 --- a/test/vpx_image_test.cc +++ b/test/vpx_image_test.cc @@ -43,3 +43,39 @@ TEST(VpxImageTest, VpxImgAllocNv12) { EXPECT_EQ(img.planes[VPX_PLANE_V], img.planes[VPX_PLANE_U] + 1); vpx_img_free(&img); } + +TEST(VpxImageTest, VpxImgAllocHugeWidth) { + // The stride (0x80000000 * 2) would overflow unsigned int. + vpx_image_t *image = + vpx_img_alloc(nullptr, VPX_IMG_FMT_I42016, 0x80000000, 1, 1); + ASSERT_EQ(image, nullptr); + + // The stride (0x80000000) would overflow int. + image = vpx_img_alloc(nullptr, VPX_IMG_FMT_I420, 0x80000000, 1, 1); + ASSERT_EQ(image, nullptr); + + image = vpx_img_alloc(nullptr, VPX_IMG_FMT_I420, 0x7ffffffe, 1, 1); + if (image) { + vpx_img_free(image); + } + + image = vpx_img_alloc(nullptr, VPX_IMG_FMT_I420, 285245883, 64, 1); + if (image) { + vpx_img_free(image); + } + + image = vpx_img_alloc(nullptr, VPX_IMG_FMT_NV12, 285245883, 64, 1); + if (image) { + vpx_img_free(image); + } + + image = vpx_img_alloc(nullptr, VPX_IMG_FMT_YV12, 285245883, 64, 1); + if (image) { + vpx_img_free(image); + } + + image = vpx_img_alloc(nullptr, VPX_IMG_FMT_I42016, 285245883, 2, 1); + if (image) { + vpx_img_free(image); + } +} diff --git a/vpx/src/vpx_image.c b/vpx/src/vpx_image.c index f9f0dd602..ab3c2b231 100644 --- a/vpx/src/vpx_image.c +++ b/vpx/src/vpx_image.c @@ -21,8 +21,9 @@ static vpx_image_t *img_alloc_helper(vpx_image_t *img, vpx_img_fmt_t fmt, unsigned int buf_align, unsigned int stride_align, unsigned char *img_data) { - unsigned int h, w, s, xcs, ycs, bps; - unsigned int stride_in_bytes; + unsigned int h, w, xcs, ycs, bps; + uint64_t s; + int stride_in_bytes; unsigned int align; if (img != NULL) memset(img, 0, sizeof(vpx_image_t)); @@ -80,9 +81,11 @@ static vpx_image_t *img_alloc_helper(vpx_image_t *img, vpx_img_fmt_t fmt, * and height shouldn't be adjusted. */ w = d_w; h = d_h; - s = (fmt & VPX_IMG_FMT_PLANAR) ? w : bps * w / 8; - s = (s + stride_align - 1) & ~(stride_align - 1); - stride_in_bytes = (fmt & VPX_IMG_FMT_HIGHBITDEPTH) ? s * 2 : s; + s = (fmt & VPX_IMG_FMT_PLANAR) ? w : (uint64_t)bps * w / 8; + s = (s + stride_align - 1) & ~((uint64_t)stride_align - 1); + s = (fmt & VPX_IMG_FMT_HIGHBITDEPTH) ? s * 2 : s; + if (s > INT_MAX) goto fail; + stride_in_bytes = (int)s; /* Allocate the new image */ if (!img) { @@ -103,9 +106,11 @@ static vpx_image_t *img_alloc_helper(vpx_image_t *img, vpx_img_fmt_t fmt, align = (1 << ycs) - 1; h = (d_h + align) & ~align; - s = (fmt & VPX_IMG_FMT_PLANAR) ? w : bps * w / 8; - s = (s + stride_align - 1) & ~(stride_align - 1); - stride_in_bytes = (fmt & VPX_IMG_FMT_HIGHBITDEPTH) ? s * 2 : s; + s = (fmt & VPX_IMG_FMT_PLANAR) ? w : (uint64_t)bps * w / 8; + s = (s + stride_align - 1) & ~((uint64_t)stride_align - 1); + s = (fmt & VPX_IMG_FMT_HIGHBITDEPTH) ? s * 2 : s; + if (s > INT_MAX) goto fail; + stride_in_bytes = (int)s; alloc_size = (fmt & VPX_IMG_FMT_PLANAR) ? (uint64_t)h * s * bps / 8 : (uint64_t)h * s; @@ -170,12 +175,12 @@ int vpx_img_set_rect(vpx_image_t *img, unsigned int x, unsigned int y, if (img->fmt & VPX_IMG_FMT_HAS_ALPHA) { img->planes[VPX_PLANE_ALPHA] = data + x * bytes_per_sample + y * img->stride[VPX_PLANE_ALPHA]; - data += img->h * img->stride[VPX_PLANE_ALPHA]; + data += (size_t)img->h * img->stride[VPX_PLANE_ALPHA]; } img->planes[VPX_PLANE_Y] = data + x * bytes_per_sample + y * img->stride[VPX_PLANE_Y]; - data += img->h * img->stride[VPX_PLANE_Y]; + data += (size_t)img->h * img->stride[VPX_PLANE_Y]; if (img->fmt == VPX_IMG_FMT_NV12) { img->planes[VPX_PLANE_U] = @@ -186,7 +191,8 @@ int vpx_img_set_rect(vpx_image_t *img, unsigned int x, unsigned int y, img->planes[VPX_PLANE_U] = data + (x >> img->x_chroma_shift) * bytes_per_sample + (y >> img->y_chroma_shift) * img->stride[VPX_PLANE_U]; - data += (img->h >> img->y_chroma_shift) * img->stride[VPX_PLANE_U]; + data += + (size_t)(img->h >> img->y_chroma_shift) * img->stride[VPX_PLANE_U]; img->planes[VPX_PLANE_V] = data + (x >> img->x_chroma_shift) * bytes_per_sample + (y >> img->y_chroma_shift) * img->stride[VPX_PLANE_V]; @@ -194,7 +200,8 @@ int vpx_img_set_rect(vpx_image_t *img, unsigned int x, unsigned int y, img->planes[VPX_PLANE_V] = data + (x >> img->x_chroma_shift) * bytes_per_sample + (y >> img->y_chroma_shift) * img->stride[VPX_PLANE_V]; - data += (img->h >> img->y_chroma_shift) * img->stride[VPX_PLANE_V]; + data += + (size_t)(img->h >> img->y_chroma_shift) * img->stride[VPX_PLANE_V]; img->planes[VPX_PLANE_U] = data + (x >> img->x_chroma_shift) * bytes_per_sample + (y >> img->y_chroma_shift) * img->stride[VPX_PLANE_U]; -- 2.30.2