From f1656def7539e2916cdf0e0a876aefcf4224f201 Mon Sep 17 00:00:00 2001 From: Tommi Date: Thu, 19 Dec 2024 15:45:27 +0100 Subject: [PATCH] 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 Commit-Queue: Tomas Gunnarsson Cr-Commit-Position: refs/heads/main@{#43617} --- api/video/BUILD.gn | 1 + api/video/i010_buffer.cc | 22 +++--- api/video/i210_buffer.cc | 21 +++--- api/video/i410_buffer.cc | 23 ++++--- api/video/i420_buffer.cc | 32 ++++----- api/video/i422_buffer.cc | 22 +++--- api/video/i444_buffer.cc | 26 +++++--- api/video/nv12_buffer.cc | 16 ++--- api/video/video_frame_buffer.cc | 12 ++++ api/video/video_frame_buffer.h | 11 +++ common_video/libyuv/libyuv_unittest.cc | 92 ++++++++++++++++++++++++-- 11 files changed, 198 insertions(+), 80 deletions(-) diff --git a/api/video/BUILD.gn b/api/video/BUILD.gn index 337e02f61c..ae763da199 100644 --- a/api/video/BUILD.gn +++ b/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", diff --git a/api/video/i010_buffer.cc b/api/video/i010_buffer.cc index b38b511c75..a7af17463c 100644 --- a/api/video/i010_buffer.cc +++ b/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) { - return kBytesPerPixel * - (stride_y * height + (stride_u + stride_v) * ((height + 1) / 2)); +int I010DataSize(int width, + 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(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( - AlignedMalloc(I010DataSize(height, stride_y, stride_u, stride_v), - kBufferAlignment))) { - RTC_DCHECK_GT(width, 0); - RTC_DCHECK_GT(height, 0); - RTC_DCHECK_GE(stride_y, width); + data_(static_cast(AlignedMalloc( + I010DataSize(width, height, stride_y, stride_u, stride_v), + kBufferAlignment))) { RTC_DCHECK_GE(stride_u, (width + 1) / 2); RTC_DCHECK_GE(stride_v, (width + 1) / 2); } diff --git a/api/video/i210_buffer.cc b/api/video/i210_buffer.cc index bb9095fbb8..05cb0ea343 100644 --- a/api/video/i210_buffer.cc +++ b/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) { - return kBytesPerPixel * - (stride_y * height + stride_u * height + stride_v * height); +int I210DataSize(int width, + 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(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( - AlignedMalloc(I210DataSize(height, stride_y, stride_u, stride_v), - kBufferAlignment))) { - RTC_DCHECK_GT(width, 0); - RTC_DCHECK_GT(height, 0); - RTC_DCHECK_GE(stride_y, width); + data_(static_cast(AlignedMalloc( + I210DataSize(width, height, stride_y, stride_u, stride_v), + kBufferAlignment))) { RTC_DCHECK_GE(stride_u, (width + 1) / 2); RTC_DCHECK_GE(stride_v, (width + 1) / 2); } diff --git a/api/video/i410_buffer.cc b/api/video/i410_buffer.cc index 07f53ce0d1..5d5e50e013 100644 --- a/api/video/i410_buffer.cc +++ b/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) { - return kBytesPerPixel * - (stride_y * height + stride_u * height + stride_v * height); +int I410DataSize(int width, + 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(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( - AlignedMalloc(I410DataSize(height, stride_y, stride_u, stride_v), - kBufferAlignment))) { - RTC_DCHECK_GT(width, 0); - RTC_DCHECK_GT(height, 0); - RTC_DCHECK_GE(stride_y, width); + data_(static_cast(AlignedMalloc( + I410DataSize(width, height, stride_y, stride_u, stride_v), + kBufferAlignment))) { RTC_DCHECK_GE(stride_u, width); RTC_DCHECK_GE(stride_v, width); } @@ -155,7 +158,7 @@ rtc::scoped_refptr 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 { diff --git a/api/video/i420_buffer.cc b/api/video/i420_buffer.cc index 5a57ee08b9..1cd8a948df 100644 --- a/api/video/i420_buffer.cc +++ b/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(int64_t height, - int64_t stride_y, - int64_t stride_u, - int64_t stride_v) { - RTC_DCHECK(height >= 0 && height <= std::numeric_limits::max()); - RTC_DCHECK(stride_y >= 0 && stride_y <= std::numeric_limits::max()); - RTC_DCHECK(stride_u >= 0 && stride_u <= std::numeric_limits::max()); - RTC_DCHECK(stride_v >= 0 && stride_v <= std::numeric_limits::max()); - return rtc::checked_cast(stride_y * height + - (stride_u + stride_v) * ((height + 1) / 2)); +int I420DataSize(int width, + int height, + int stride_y, + int stride_u, + int stride_v) { + CheckValidDimensions(width, height, stride_y, stride_u, stride_v); + // Do the size calculation using 64bit integers and use checked_cast to catch + // overflow. + int64_t h = height, y = stride_y, u = stride_u, v = stride_v; + return rtc::checked_cast(y * h + (u + v) * ((h + 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( - AlignedMalloc(I420DataSize(height, stride_y, stride_u, stride_v), - kBufferAlignment))) { - RTC_DCHECK_GT(width, 0); - RTC_DCHECK_GT(height, 0); - RTC_DCHECK_GE(stride_y, width); + data_(static_cast(AlignedMalloc( + I420DataSize(width, height, stride_y, stride_u, stride_v), + kBufferAlignment))) { RTC_DCHECK_GE(stride_u, (width + 1) / 2); RTC_DCHECK_GE(stride_v, (width + 1) / 2); } @@ -148,7 +144,7 @@ rtc::scoped_refptr 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 { diff --git a/api/video/i422_buffer.cc b/api/video/i422_buffer.cc index a624c19049..725f232375 100644 --- a/api/video/i422_buffer.cc +++ b/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) { - return stride_y * height + stride_u * height + stride_v * height; +int I422DataSize(int width, + 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(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( - AlignedMalloc(I422DataSize(height, stride_y, stride_u, stride_v), - kBufferAlignment))) { - RTC_DCHECK_GT(width, 0); - RTC_DCHECK_GT(height, 0); - RTC_DCHECK_GE(stride_y, width); + data_(static_cast(AlignedMalloc( + I422DataSize(width, height, stride_y, stride_u, stride_v), + kBufferAlignment))) { RTC_DCHECK_GE(stride_u, (width + 1) / 2); RTC_DCHECK_GE(stride_v, (width + 1) / 2); } @@ -169,7 +173,7 @@ rtc::scoped_refptr 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 { diff --git a/api/video/i444_buffer.cc b/api/video/i444_buffer.cc index 9120dcc842..b27ba223b4 100644 --- a/api/video/i444_buffer.cc +++ b/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) { - return stride_y * height + stride_u * height + stride_v * height; +int I444DataSize(int width, + 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(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( - AlignedMalloc(I444DataSize(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)); + data_(static_cast(AlignedMalloc( + I444DataSize(width, height, stride_y, stride_u, stride_v), + kBufferAlignment))) { + RTC_DCHECK_GE(stride_u, width); + RTC_DCHECK_GE(stride_v, width); } I444Buffer::~I444Buffer() {} @@ -149,7 +153,7 @@ rtc::scoped_refptr 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 { diff --git a/api/video/nv12_buffer.cc b/api/video/nv12_buffer.cc index 93ea0e4789..5979da0649 100644 --- a/api/video/nv12_buffer.cc +++ b/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) { - return stride_y * height + stride_uv * ((height + 1) / 2); +int NV12DataSize(int width, int height, int stride_y, int stride_uv) { + CheckValidDimensions(width, height, stride_y, stride_uv, stride_uv); + int64_t h = height, y = stride_y, uv = stride_uv; + return rtc::checked_cast(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( - AlignedMalloc(NV12DataSize(height_, stride_y_, stride_uv), + AlignedMalloc(NV12DataSize(width, height, stride_y, stride_uv), kBufferAlignment))) { - RTC_DCHECK_GT(width, 0); - RTC_DCHECK_GT(height, 0); - RTC_DCHECK_GE(stride_y, width); - RTC_DCHECK_GE(stride_uv, (width + width % 2)); + 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, diff --git a/api/video/video_frame_buffer.cc b/api/video/video_frame_buffer.cc index 202dd0d634..73da862965 100644 --- a/api/video/video_frame_buffer.cc +++ b/api/video/video_frame_buffer.cc @@ -247,4 +247,16 @@ rtc::scoped_refptr 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 diff --git a/api/video/video_frame_buffer.h b/api/video/video_frame_buffer.h index bf0e83e018..f206600a3b 100644 --- a/api/video/video_frame_buffer.h +++ b/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::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_ diff --git a/common_video/libyuv/libyuv_unittest.cc b/common_video/libyuv/libyuv_unittest.cc index 6d569c0e39..014dc89954 100644 --- a/common_video/libyuv/libyuv_unittest.cc +++ b/common_video/libyuv/libyuv_unittest.cc @@ -15,7 +15,13 @@ #include +#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::max(), ""); - // The Y plane is the image width * height, while the strides of the U and V - // planes are half the width. - const int stride_uv = (kWidth + 1) / 2; - EXPECT_DEATH(I420Buffer::Create(kWidth, kHeight, /*stride_y=*/kWidth, - /*stride_u=*/stride_uv, - /*stride_v=*/stride_uv), + EXPECT_DEATH(I010Buffer::Create(kWidth, kHeight), "IsValueInRangeForNumericType"); + 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_uv, 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 +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 +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 +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(); + TestInvalidDimensions2Param(); + // `Create() is provided with width, height, y, u, v. + TestInvalidDimensions5Params(); + TestInvalidDimensions5Params(); + TestInvalidDimensions5Params(); + TestInvalidDimensions5Params(); + // `Create() is provided with width, height, y, u_and_v. + TestInvalidDimensions4Params(); +} + #endif // GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID) } // namespace webrtc