Apply dimension checking across the YUV buffer classes.

Bug: chromium:371686447
Change-Id: I15502748c0b0036aaef3742bce27104887b77f65
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/372140
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43617}

api/video/BUILD.gn

@@ -102,6 +102,7 @@ rtc_library("video_frame_i010") {
     "..:scoped_refptr",
     "../../rtc_base:checks",
     "../../rtc_base:refcount",
+    "../../rtc_base:safe_conversions",
     "../../rtc_base/memory:aligned_malloc",
     "../../rtc_base/system:rtc_export",
     "//third_party/libyuv",

api/video/i010_buffer.cc

@@ -19,6 +19,7 @@
 #include "api/video/video_rotation.h"
 #include "rtc_base/checks.h"
 #include "rtc_base/memory/aligned_malloc.h"
+#include "rtc_base/numerics/safe_conversions.h"
 #include "third_party/libyuv/include/libyuv/convert.h"
 #include "third_party/libyuv/include/libyuv/convert_from.h"
 #include "third_party/libyuv/include/libyuv/rotate.h"
@@ -32,9 +33,15 @@ namespace webrtc {
 
 namespace {
 
-int I010DataSize(int height, int stride_y, int stride_u, int stride_v) {
+int I010DataSize(int width,
-  return kBytesPerPixel *
+                 int height,
-         (stride_y * height + (stride_u + stride_v) * ((height + 1) / 2));
+                 int stride_y,
+                 int stride_u,
+                 int stride_v) {
+  CheckValidDimensions(width, height, stride_y, stride_u, stride_v);
+  int64_t h = height, y = stride_y, u = stride_u, v = stride_v;
+  return rtc::checked_cast<int>(kBytesPerPixel *
+                                (y * h + (u + v) * ((h + 1) / 2)));
 }
 
 }  // namespace
@@ -49,12 +56,9 @@ I010Buffer::I010Buffer(int width,
       stride_y_(stride_y),
       stride_u_(stride_u),
       stride_v_(stride_v),
-      data_(static_cast<uint16_t*>(
+      data_(static_cast<uint16_t*>(AlignedMalloc(
-          AlignedMalloc(I010DataSize(height, stride_y, stride_u, stride_v),
+          I010DataSize(width, height, stride_y, stride_u, stride_v),
           kBufferAlignment))) {
-  RTC_DCHECK_GT(width, 0);
-  RTC_DCHECK_GT(height, 0);
-  RTC_DCHECK_GE(stride_y, width);
   RTC_DCHECK_GE(stride_u, (width + 1) / 2);
   RTC_DCHECK_GE(stride_v, (width + 1) / 2);
 }

api/video/i210_buffer.cc

@@ -20,6 +20,7 @@
 #include "api/video/video_rotation.h"
 #include "rtc_base/checks.h"
 #include "rtc_base/memory/aligned_malloc.h"
+#include "rtc_base/numerics/safe_conversions.h"
 #include "third_party/libyuv/include/libyuv/convert.h"
 #include "third_party/libyuv/include/libyuv/planar_functions.h"
 #include "third_party/libyuv/include/libyuv/rotate.h"
@@ -33,9 +34,14 @@ namespace webrtc {
 
 namespace {
 
-int I210DataSize(int height, int stride_y, int stride_u, int stride_v) {
+int I210DataSize(int width,
-  return kBytesPerPixel *
+                 int height,
-         (stride_y * height + stride_u * height + stride_v * height);
+                 int stride_y,
+                 int stride_u,
+                 int stride_v) {
+  CheckValidDimensions(width, height, stride_y, stride_u, stride_v);
+  int64_t h = height, y = stride_y, u = stride_u, v = stride_v;
+  return rtc::checked_cast<int>(kBytesPerPixel * (y * h + u * h + v * h));
 }
 
 }  // namespace
@@ -50,12 +56,9 @@ I210Buffer::I210Buffer(int width,
       stride_y_(stride_y),
       stride_u_(stride_u),
       stride_v_(stride_v),
-      data_(static_cast<uint16_t*>(
+      data_(static_cast<uint16_t*>(AlignedMalloc(
-          AlignedMalloc(I210DataSize(height, stride_y, stride_u, stride_v),
+          I210DataSize(width, height, stride_y, stride_u, stride_v),
           kBufferAlignment))) {
-  RTC_DCHECK_GT(width, 0);
-  RTC_DCHECK_GT(height, 0);
-  RTC_DCHECK_GE(stride_y, width);
   RTC_DCHECK_GE(stride_u, (width + 1) / 2);
   RTC_DCHECK_GE(stride_v, (width + 1) / 2);
 }

api/video/i410_buffer.cc

@@ -22,6 +22,7 @@
 #include "api/video/video_rotation.h"
 #include "rtc_base/checks.h"
 #include "rtc_base/memory/aligned_malloc.h"
+#include "rtc_base/numerics/safe_conversions.h"
 #include "third_party/libyuv/include/libyuv/convert.h"
 #include "third_party/libyuv/include/libyuv/planar_functions.h"
 #include "third_party/libyuv/include/libyuv/rotate.h"
@@ -35,9 +36,14 @@ namespace webrtc {
 
 namespace {
 
-int I410DataSize(int height, int stride_y, int stride_u, int stride_v) {
+int I410DataSize(int width,
-  return kBytesPerPixel *
+                 int height,
-         (stride_y * height + stride_u * height + stride_v * height);
+                 int stride_y,
+                 int stride_u,
+                 int stride_v) {
+  CheckValidDimensions(width, height, stride_y, stride_u, stride_v);
+  int64_t h = height, y = stride_y, u = stride_u, v = stride_v;
+  return rtc::checked_cast<int>(kBytesPerPixel * (y * h + u * h + v * h));
 }
 
 }  // namespace
@@ -55,12 +61,9 @@ I410Buffer::I410Buffer(int width,
       stride_y_(stride_y),
       stride_u_(stride_u),
       stride_v_(stride_v),
-      data_(static_cast<uint16_t*>(
+      data_(static_cast<uint16_t*>(AlignedMalloc(
-          AlignedMalloc(I410DataSize(height, stride_y, stride_u, stride_v),
+          I410DataSize(width, height, stride_y, stride_u, stride_v),
           kBufferAlignment))) {
-  RTC_DCHECK_GT(width, 0);
-  RTC_DCHECK_GT(height, 0);
-  RTC_DCHECK_GE(stride_y, width);
   RTC_DCHECK_GE(stride_u, width);
   RTC_DCHECK_GE(stride_v, width);
 }
@@ -155,7 +158,7 @@ rtc::scoped_refptr<I420BufferInterface> I410Buffer::ToI420() {
 
 void I410Buffer::InitializeData() {
   memset(data_.get(), 0,
-         I410DataSize(height_, stride_y_, stride_u_, stride_v_));
+         I410DataSize(width_, height_, stride_y_, stride_u_, stride_v_));
 }
 
 int I410Buffer::width() const {

api/video/i420_buffer.cc

@@ -35,17 +35,16 @@ namespace webrtc {
 
 namespace {
 
-// Do the size calculation using 64bit integers and check for int overflow.
+int I420DataSize(int width,
-int I420DataSize(int64_t height,
+                 int height,
-                 int64_t stride_y,
+                 int stride_y,
-                 int64_t stride_u,
+                 int stride_u,
-                 int64_t stride_v) {
+                 int stride_v) {
-  RTC_DCHECK(height >= 0 && height <= std::numeric_limits<int>::max());
+  CheckValidDimensions(width, height, stride_y, stride_u, stride_v);
-  RTC_DCHECK(stride_y >= 0 && stride_y <= std::numeric_limits<int>::max());
+  // Do the size calculation using 64bit integers and use checked_cast to catch
-  RTC_DCHECK(stride_u >= 0 && stride_u <= std::numeric_limits<int>::max());
+  // overflow.
-  RTC_DCHECK(stride_v >= 0 && stride_v <= std::numeric_limits<int>::max());
+  int64_t h = height, y = stride_y, u = stride_u, v = stride_v;
-  return rtc::checked_cast<int>(stride_y * height +
+  return rtc::checked_cast<int>(y * h + (u + v) * ((h + 1) / 2));
-                                (stride_u + stride_v) * ((height + 1) / 2));
 }
 
 }  // namespace
@@ -63,12 +62,9 @@ I420Buffer::I420Buffer(int width,
       stride_y_(stride_y),
       stride_u_(stride_u),
       stride_v_(stride_v),
-      data_(static_cast<uint8_t*>(
+      data_(static_cast<uint8_t*>(AlignedMalloc(
-          AlignedMalloc(I420DataSize(height, stride_y, stride_u, stride_v),
+          I420DataSize(width, height, stride_y, stride_u, stride_v),
           kBufferAlignment))) {
-  RTC_DCHECK_GT(width, 0);
-  RTC_DCHECK_GT(height, 0);
-  RTC_DCHECK_GE(stride_y, width);
   RTC_DCHECK_GE(stride_u, (width + 1) / 2);
   RTC_DCHECK_GE(stride_v, (width + 1) / 2);
 }
@@ -148,7 +144,7 @@ rtc::scoped_refptr<I420Buffer> I420Buffer::Rotate(
 
 void I420Buffer::InitializeData() {
   memset(data_.get(), 0,
-         I420DataSize(height_, stride_y_, stride_u_, stride_v_));
+         I420DataSize(width_, height_, stride_y_, stride_u_, stride_v_));
 }
 
 int I420Buffer::width() const {

api/video/i422_buffer.cc

@@ -22,6 +22,7 @@
 #include "api/video/video_rotation.h"
 #include "rtc_base/checks.h"
 #include "rtc_base/memory/aligned_malloc.h"
+#include "rtc_base/numerics/safe_conversions.h"
 #include "third_party/libyuv/include/libyuv/convert.h"
 #include "third_party/libyuv/include/libyuv/convert_from.h"
 #include "third_party/libyuv/include/libyuv/planar_functions.h"
@@ -35,8 +36,14 @@ namespace webrtc {
 
 namespace {
 
-int I422DataSize(int height, int stride_y, int stride_u, int stride_v) {
+int I422DataSize(int width,
-  return stride_y * height + stride_u * height + stride_v * height;
+                 int height,
+                 int stride_y,
+                 int stride_u,
+                 int stride_v) {
+  CheckValidDimensions(width, height, stride_y, stride_u, stride_v);
+  int64_t h = height, y = stride_y, u = stride_u, v = stride_v;
+  return rtc::checked_cast<int>(y * h + u * h + v * h);
 }
 }  // namespace
 
@@ -53,12 +60,9 @@ I422Buffer::I422Buffer(int width,
       stride_y_(stride_y),
       stride_u_(stride_u),
       stride_v_(stride_v),
-      data_(static_cast<uint8_t*>(
+      data_(static_cast<uint8_t*>(AlignedMalloc(
-          AlignedMalloc(I422DataSize(height, stride_y, stride_u, stride_v),
+          I422DataSize(width, height, stride_y, stride_u, stride_v),
           kBufferAlignment))) {
-  RTC_DCHECK_GT(width, 0);
-  RTC_DCHECK_GT(height, 0);
-  RTC_DCHECK_GE(stride_y, width);
   RTC_DCHECK_GE(stride_u, (width + 1) / 2);
   RTC_DCHECK_GE(stride_v, (width + 1) / 2);
 }
@@ -169,7 +173,7 @@ rtc::scoped_refptr<I420BufferInterface> I422Buffer::ToI420() {
 
 void I422Buffer::InitializeData() {
   memset(data_.get(), 0,
-         I422DataSize(height_, stride_y_, stride_u_, stride_v_));
+         I422DataSize(width_, height_, stride_y_, stride_u_, stride_v_));
 }
 
 int I422Buffer::width() const {

api/video/i444_buffer.cc

@@ -22,6 +22,7 @@
 #include "api/video/video_rotation.h"
 #include "rtc_base/checks.h"
 #include "rtc_base/memory/aligned_malloc.h"
+#include "rtc_base/numerics/safe_conversions.h"
 #include "third_party/libyuv/include/libyuv/convert.h"
 #include "third_party/libyuv/include/libyuv/planar_functions.h"
 #include "third_party/libyuv/include/libyuv/rotate.h"
@@ -34,8 +35,14 @@ namespace webrtc {
 
 namespace {
 
-int I444DataSize(int height, int stride_y, int stride_u, int stride_v) {
+int I444DataSize(int width,
-  return stride_y * height + stride_u * height + stride_v * height;
+                 int height,
+                 int stride_y,
+                 int stride_u,
+                 int stride_v) {
+  CheckValidDimensions(width, height, stride_y, stride_u, stride_v);
+  int64_t h = height, y = stride_y, u = stride_u, v = stride_v;
+  return rtc::checked_cast<int>(y * h + u * h + v * h);
 }
 
 }  // namespace
@@ -53,14 +60,11 @@ I444Buffer::I444Buffer(int width,
       stride_y_(stride_y),
       stride_u_(stride_u),
       stride_v_(stride_v),
-      data_(static_cast<uint8_t*>(
+      data_(static_cast<uint8_t*>(AlignedMalloc(
-          AlignedMalloc(I444DataSize(height, stride_y, stride_u, stride_v),
+          I444DataSize(width, height, stride_y, stride_u, stride_v),
           kBufferAlignment))) {
-  RTC_DCHECK_GT(width, 0);
+  RTC_DCHECK_GE(stride_u, width);
-  RTC_DCHECK_GT(height, 0);
+  RTC_DCHECK_GE(stride_v, width);
-  RTC_DCHECK_GE(stride_y, width);
-  RTC_DCHECK_GE(stride_u, (width));
-  RTC_DCHECK_GE(stride_v, (width));
 }
 
 I444Buffer::~I444Buffer() {}
@@ -149,7 +153,7 @@ rtc::scoped_refptr<I420BufferInterface> I444Buffer::ToI420() {
 
 void I444Buffer::InitializeData() {
   memset(data_.get(), 0,
-         I444DataSize(height_, stride_y_, stride_u_, stride_v_));
+         I444DataSize(width_, height_, stride_y_, stride_u_, stride_v_));
 }
 
 int I444Buffer::width() const {

api/video/nv12_buffer.cc

@@ -20,6 +20,7 @@
 #include "api/video/video_frame_buffer.h"
 #include "rtc_base/checks.h"
 #include "rtc_base/memory/aligned_malloc.h"
+#include "rtc_base/numerics/safe_conversions.h"
 #include "third_party/libyuv/include/libyuv/convert.h"
 #include "third_party/libyuv/include/libyuv/scale.h"
 
@@ -29,8 +30,10 @@ namespace {
 
 static const int kBufferAlignment = 64;
 
-int NV12DataSize(int height, int stride_y, int stride_uv) {
+int NV12DataSize(int width, int height, int stride_y, int stride_uv) {
-  return stride_y * height + stride_uv * ((height + 1) / 2);
+  CheckValidDimensions(width, height, stride_y, stride_uv, stride_uv);
+  int64_t h = height, y = stride_y, uv = stride_uv;
+  return rtc::checked_cast<int>(y * h + uv * ((h + 1) / 2));
 }
 
 }  // namespace
@@ -44,12 +47,9 @@ NV12Buffer::NV12Buffer(int width, int height, int stride_y, int stride_uv)
       stride_y_(stride_y),
       stride_uv_(stride_uv),
       data_(static_cast<uint8_t*>(
-          AlignedMalloc(NV12DataSize(height_, stride_y_, stride_uv),
+          AlignedMalloc(NV12DataSize(width, height, stride_y, stride_uv),
                         kBufferAlignment))) {
-  RTC_DCHECK_GT(width, 0);
+  RTC_DCHECK_GE(stride_uv, width + width % 2);
-  RTC_DCHECK_GT(height, 0);
-  RTC_DCHECK_GE(stride_y, width);
-  RTC_DCHECK_GE(stride_uv, (width + width % 2));
 }
 
 NV12Buffer::~NV12Buffer() = default;
@@ -126,7 +126,7 @@ size_t NV12Buffer::UVOffset() const {
 }
 
 void NV12Buffer::InitializeData() {
-  memset(data_.get(), 0, NV12DataSize(height_, stride_y_, stride_uv_));
+  memset(data_.get(), 0, NV12DataSize(width_, height_, stride_y_, stride_uv_));
 }
 
 void NV12Buffer::CropAndScaleFrom(const NV12BufferInterface& src,

api/video/video_frame_buffer.cc

@@ -247,4 +247,16 @@ rtc::scoped_refptr<VideoFrameBuffer> NV12BufferInterface::CropAndScale(
   return result;
 }
 
+void CheckValidDimensions(int width,
+                          int height,
+                          int stride_y,
+                          int stride_u,
+                          int stride_v) {
+  RTC_CHECK_GT(width, 0);
+  RTC_CHECK_GT(height, 0);
+  RTC_CHECK_GE(stride_y, width);
+  RTC_CHECK_GT(stride_u, 0);
+  RTC_CHECK_GT(stride_v, 0);
+}
+
 }  // namespace webrtc

api/video/video_frame_buffer.h

@@ -324,6 +324,17 @@ class RTC_EXPORT NV12BufferInterface : public BiplanarYuv8Buffer {
   ~NV12BufferInterface() override {}
 };
 
+// RTC_CHECKs that common values used to calculate buffer sizes are within the
+// range of [1..std::numeric_limits<int>::max()].
+// `width` and `height` must be > 0, `stride_y` must be >= `width` whereas
+// `stride_u` and `stride_v` must be `> 0` as this is where the various yuv
+// formats differ.
+void CheckValidDimensions(int width,
+                          int height,
+                          int stride_y,
+                          int stride_u,
+                          int stride_v);
+
 }  // namespace webrtc
 
 #endif  // API_VIDEO_VIDEO_FRAME_BUFFER_H_

common_video/libyuv/libyuv_unittest.cc

@@ -15,7 +15,13 @@
 
 #include <memory>
 
+#include "api/video/i010_buffer.h"
+#include "api/video/i210_buffer.h"
+#include "api/video/i410_buffer.h"
 #include "api/video/i420_buffer.h"
+#include "api/video/i422_buffer.h"
+#include "api/video/i444_buffer.h"
+#include "api/video/nv12_buffer.h"
 #include "api/video/video_frame.h"
 #include "common_video/libyuv/include/webrtc_libyuv.h"
 #include "rtc_base/logging.h"
@@ -397,14 +403,88 @@ TEST_F(TestLibYuv, I420DimensionsTooLarge) {
       (int64_t{kWidth} * int64_t{kHeight}) > std::numeric_limits<int>::max(),
       "");
 
-  // The Y plane is the image width * height, while the strides of the U and V
+  EXPECT_DEATH(I010Buffer::Create(kWidth, kHeight),
-  // planes are half the width.
+               "IsValueInRangeForNumericType");
-  const int stride_uv = (kWidth + 1) / 2;
+  EXPECT_DEATH(I210Buffer::Create(kWidth, kHeight),
+               "IsValueInRangeForNumericType");
+
+  int stride_uv = (kWidth + 1) / 2;
+  EXPECT_DEATH(I410Buffer::Create(kWidth, kHeight, /*stride_y=*/kWidth,
+                                  stride_uv, stride_uv),
+               "IsValueInRangeForNumericType");
   EXPECT_DEATH(I420Buffer::Create(kWidth, kHeight, /*stride_y=*/kWidth,
-                                  /*stride_u=*/stride_uv,
+                                  stride_uv, stride_uv),
-                                  /*stride_v=*/stride_uv),
+               "IsValueInRangeForNumericType");
+  EXPECT_DEATH(I422Buffer::Create(kWidth, kHeight, /*stride_y=*/kWidth,
+                                  stride_uv, stride_uv),
+               "IsValueInRangeForNumericType");
+  EXPECT_DEATH(I444Buffer::Create(kWidth, kHeight, /*stride_y=*/kWidth,
+                                  stride_uv, stride_uv),
+               "IsValueInRangeForNumericType");
+  EXPECT_DEATH(
+      NV12Buffer::Create(kWidth, kHeight, /*stride_y=*/kWidth, stride_uv),
       "IsValueInRangeForNumericType");
 }
+
+template <typename T>
+void TestInvalidDimensions5Params() {
+  EXPECT_DEATH(T::Create(-11, 1, /*stride_y=*/1,
+                         /*stride_u=*/1,
+                         /*stride_v=*/1),
+               "> 0");
+  EXPECT_DEATH(T::Create(1, -11, /*stride_y=*/1,
+                         /*stride_u=*/1,
+                         /*stride_v=*/1),
+               "> 0");
+  EXPECT_DEATH(T::Create(1, 1, /*stride_y=*/-12,
+                         /*stride_u=*/1,
+                         /*stride_v=*/1),
+               ">= width");
+  EXPECT_DEATH(T::Create(1, 1, /*stride_y=*/1,
+                         /*stride_u=*/-12,
+                         /*stride_v=*/1),
+               "> 0");
+  EXPECT_DEATH(T::Create(1, 1, /*stride_y=*/1,
+                         /*stride_u=*/1,
+                         /*stride_v=*/-12),
+               "> 0");
+}
+
+template <typename T>
+void TestInvalidDimensions4Params() {
+  EXPECT_DEATH(T::Create(-11, 1, /*stride_y=*/1,
+                         /*stride_uv=*/1),
+               "> 0");
+  EXPECT_DEATH(T::Create(1, -11, /*stride_y=*/1,
+                         /*stride_uv=*/1),
+               "> 0");
+  EXPECT_DEATH(T::Create(1, 1, /*stride_y=*/-12,
+                         /*stride_uv=*/1),
+               ">= width");
+  EXPECT_DEATH(T::Create(1, 1, /*stride_y=*/1,
+                         /*stride_uv=*/-12),
+               "> 0");
+}
+
+template <typename T>
+void TestInvalidDimensions2Param() {
+  EXPECT_DEATH(T::Create(-11, 1), "> 0");
+  EXPECT_DEATH(T::Create(1, -11), "> 0");
+}
+
+TEST_F(TestLibYuv, I420InvalidDimensions) {
+  // Only width and height provided to `Create()`.
+  TestInvalidDimensions2Param<I010Buffer>();
+  TestInvalidDimensions2Param<I210Buffer>();
+  // `Create() is provided with width, height, y, u, v.
+  TestInvalidDimensions5Params<I410Buffer>();
+  TestInvalidDimensions5Params<I420Buffer>();
+  TestInvalidDimensions5Params<I422Buffer>();
+  TestInvalidDimensions5Params<I444Buffer>();
+  // `Create() is provided with width, height, y, u_and_v.
+  TestInvalidDimensions4Params<NV12Buffer>();
+}
+
 #endif  // GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
 
 }  // namespace webrtc