1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
|
From 8156fb76d88845d716867d20333fd27001be47a8 Mon Sep 17 00:00:00 2001
From: Wan-Teh Chang <wtc@google.com>
Date: Thu, 4 Apr 2024 15:14:08 -0700
Subject: [PATCH 2/2] Avoid integer overflows in align_image_dimension()
Impose maximum values on the input parameters so that we can perform
arithmetic operations without worrying about overflows.
Fix a bug (introduced in commit 7aa2edc) that the ~ operator is applied
to (stride_align - 1), which is unsigned int, and then the result is
converted to uint64_t.
Also change the AomImageTest.AomImgAllocHugeWidth 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: I634c38c35a296b5bbf3de7ddf10040e7ec5ee9a1
(cherry picked from commit 60653dff7f8ee3e769a0aeec5e210a4fc2687717)
---
aom/aom_image.h | 27 ++++++++++++++++++---------
aom/src/aom_image.c | 19 +++++++++++++++----
test/aom_image_test.cc | 29 +++++++++++++++++++++++++++++
3 files changed, 62 insertions(+), 13 deletions(-)
diff --git a/aom/aom_image.h b/aom/aom_image.h
index d5f0c08..11b668c 100644
--- a/aom/aom_image.h
+++ b/aom/aom_image.h
@@ -244,10 +244,13 @@ typedef struct aom_image {
* 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
@@ -267,10 +270,12 @@ aom_image_t *aom_img_alloc(aom_image_t *img, aom_img_fmt_t fmt,
* 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 each row in the image
- * (stride).
+ * (stride). 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
@@ -291,12 +296,16 @@ aom_image_t *aom_img_wrap(aom_image_t *img, aom_img_fmt_t fmt, unsigned int d_w,
* 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
+ * (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.
* \param[in] size_align Alignment, in pixels, of the image width and height.
+ * Must not exceed 65536.
* \param[in] border A border that is padded on four sides of the image.
+ * 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
diff --git a/aom/src/aom_image.c b/aom/src/aom_image.c
index acd3694..ca5e58c 100644
--- a/aom/src/aom_image.c
+++ b/aom/src/aom_image.c
@@ -9,6 +9,7 @@
* PATENTS file, you can obtain it at www.aomedia.org/license/patent.
*/
+#include <assert.h>
#include <limits.h>
#include <stdlib.h>
#include <string.h>
@@ -36,10 +37,18 @@ static aom_image_t *img_alloc_helper(
/* NOTE: In this function, bit_depth is either 8 or 16 (if
* AOM_IMG_FMT_HIGHBITDEPTH is set), never 10 or 12.
*/
- unsigned int h, w, xcs, ycs, bps, bit_depth;
+ unsigned int xcs, ycs, bps, bit_depth;
if (img != NULL) memset(img, 0, sizeof(aom_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 || size_align > 65536 || border > 65536) {
+ goto fail;
+ }
+
/* Treat align==0 like align==1 */
if (!buf_align) buf_align = 1;
@@ -102,11 +111,13 @@ static aom_image_t *img_alloc_helper(
}
/* Calculate storage sizes given the chroma subsampling */
- w = align_image_dimension(d_w, xcs, size_align);
- h = align_image_dimension(d_h, ycs, size_align);
+ const unsigned int w = align_image_dimension(d_w, xcs, size_align);
+ assert(d_w <= w);
+ const unsigned int h = align_image_dimension(d_h, ycs, size_align);
+ assert(d_h <= h);
uint64_t s = (fmt & AOM_IMG_FMT_PLANAR) ? w : (uint64_t)bps * w / bit_depth;
- s = (s + 2 * border + stride_align - 1) & ~(stride_align - 1);
+ s = (s + 2 * border + stride_align - 1) & ~((uint64_t)stride_align - 1);
s = s * bit_depth / 8;
if (s > INT_MAX) goto fail;
const int stride_in_bytes = (int)s;
diff --git a/test/aom_image_test.cc b/test/aom_image_test.cc
index 69b777b..2b8bdfc 100644
--- a/test/aom_image_test.cc
+++ b/test/aom_image_test.cc
@@ -9,6 +9,8 @@
* PATENTS file, you can obtain it at www.aomedia.org/license/patent.
*/
+#include <climits>
+
#include "aom/aom_image.h"
#include "third_party/googletest/src/googletest/include/gtest/gtest.h"
@@ -71,6 +73,20 @@ TEST(AomImageTest, AomImgAllocHugeWidth) {
image = aom_img_alloc(nullptr, AOM_IMG_FMT_I420, 0x80000000, 1, 1);
ASSERT_EQ(image, nullptr);
+ // The aligned width (UINT_MAX + 1) would overflow unsigned int.
+ image = aom_img_alloc(nullptr, AOM_IMG_FMT_I420, UINT_MAX, 1, 1);
+ ASSERT_EQ(image, nullptr);
+
+ image = aom_img_alloc_with_border(nullptr, AOM_IMG_FMT_I422, 1, INT_MAX, 1,
+ 0x40000000, 0);
+ if (image) {
+ uint16_t *y_plane =
+ reinterpret_cast<uint16_t *>(image->planes[AOM_PLANE_Y]);
+ y_plane[0] = 0;
+ y_plane[image->d_w - 1] = 0;
+ aom_img_free(image);
+ }
+
image = aom_img_alloc(nullptr, AOM_IMG_FMT_I420, 0x7ffffffe, 1, 1);
if (image) {
aom_img_free(image);
@@ -91,8 +107,21 @@ TEST(AomImageTest, AomImgAllocHugeWidth) {
aom_img_free(image);
}
+ image = aom_img_alloc(nullptr, AOM_IMG_FMT_I42016, 65536, 2, 1);
+ if (image) {
+ uint16_t *y_plane =
+ reinterpret_cast<uint16_t *>(image->planes[AOM_PLANE_Y]);
+ y_plane[0] = 0;
+ y_plane[image->d_w - 1] = 0;
+ aom_img_free(image);
+ }
+
image = aom_img_alloc(nullptr, AOM_IMG_FMT_I42016, 285245883, 2, 1);
if (image) {
+ uint16_t *y_plane =
+ reinterpret_cast<uint16_t *>(image->planes[AOM_PLANE_Y]);
+ y_plane[0] = 0;
+ y_plane[image->d_w - 1] = 0;
aom_img_free(image);
}
}
--
2.41.0
|
