Skip to content

Commit bc7c56e

Browse files
lgritzlgritz
committed
fix(dps): Several safety fixes for corrupt files
* SWAPRGBABytes convert to span-based * check_open for dpx files to check for reasonable/legal resolutions and channel counts * comment out function declarations not used by OIIO * overflow safety, replace int with size_t and use safe_mult64 Signed-off-by: Larry Gritz <lg@larrygritz.com>
1 parent 3c2e174 commit bc7c56e

3 files changed

Lines changed: 57 additions & 43 deletions

File tree

‎src/dpx.imageio/dpxinput.cpp‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,8 @@ DPXInput::seek_subimage(int subimage, int miplevel)
208208
m_spec = ImageSpec(m_dpx.header.Width(), m_dpx.header.Height(),
209209
m_dpx.header.ImageElementComponentCount(subimage),
210210
typedesc);
211+
if (!check_open(m_spec, { 0, 1 << 20, 0, 1 << 20, 0, 1 << 16, 0, 8 }))
212+
return false;
211213

212214
// xOffset/yOffset are defined as unsigned 32-bit integers, but m_spec.x/y are signed
213215
// avoid casts that would result in negative values
@@ -599,7 +601,8 @@ DPXInput::read_native_scanlines(int subimage, int miplevel, int ybegin,
599601
} else {
600602
// read the scanline and convert to RGB
601603
unsigned char* ptr = (unsigned char*)data;
602-
int bufsize = dpx::QueryRGBBufferSize(m_dpx.header, subimage, block);
604+
int64_t bufsize = dpx::QueryRGBBufferSize(m_dpx.header, subimage,
605+
block);
603606
if (bufsize > 0) {
604607
m_decodebuf.resize(bufsize);
605608
ptr = m_decodebuf.data();

‎src/dpx.imageio/libdpx/DPXColorConverter.cpp‎

Lines changed: 46 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -34,19 +34,23 @@
3434

3535

3636
#include "DPXColorConverter.h"
37+
#include <OpenImageIO/fmath.h>
38+
#include <OpenImageIO/span.h>
3739
#include <algorithm>
3840

3941
namespace dpx {
4042
template <typename DATA>
41-
static inline bool SwapRGBABytes(const DATA *input, DATA *output, int pixels) {
42-
// copy the data that could be destroyed to an additional buffer in case input == output
43-
DATA tmp[2];
44-
for (int i = 0; i < pixels; i++) {
45-
memcpy(tmp, &input[i * 4], sizeof(DATA) * 2);
46-
output[i * 4 + 0] = input[i * 4 + 3];
47-
output[i * 4 + 1] = input[i * 4 + 2];
48-
output[i * 4 + 2] = tmp[1];
49-
output[i * 4 + 3] = tmp[0];
43+
static inline bool SwapRGBABytes(OIIO::cspan<DATA> input, OIIO::span<DATA> output) {
44+
size_t pixels = input.size() / 4;
45+
if (pixels * 4 + 3 >= input.size() || pixels * 4 + 3 >= output.size())
46+
return false;
47+
// Because we checked the lengths, we can just use data pointers below
48+
for (size_t i = 0; i < pixels; i++) {
49+
DATA a = input.data()[i * 4 + 0], b = input.data()[i * 4 + 1];
50+
output.data()[i * 4 + 0] = input.data()[i * 4 + 3];
51+
output.data()[i * 4 + 1] = input.data()[i * 4 + 2];
52+
output.data()[i * 4 + 2] = b;
53+
output.data()[i * 4 + 3] = a;
5054
}
5155
return true;
5256
}
@@ -107,12 +111,12 @@ namespace dpx {
107111

108112
// 4:4:4
109113
template <typename DATA, unsigned int max>
110-
static bool ConvertCbYCrToRGB(const Characteristic space, const DATA *input, DATA *output, const int pixels) {
114+
static bool ConvertCbYCrToRGB(const Characteristic space, const DATA *input, DATA *output, const size_t pixels) {
111115
const float *matrix = GetYCbCrToRGBColorMatrix(space);
112116
if (matrix == NULL)
113117
return false;
114118
DATA RGB[3];
115-
for (int i = 0; i < pixels; i++) {
119+
for (size_t i = 0; i < pixels; i++) {
116120
ConvertPixelYCbCrToRGB<DATA, max>(&input[i * 3], RGB, matrix);
117121
memcpy(&output[i * 3], RGB, sizeof(DATA) * 3);
118122
}
@@ -121,12 +125,12 @@ namespace dpx {
121125

122126
// 4:4:4:4
123127
template <typename DATA, unsigned int max>
124-
static bool ConvertCbYCrAToRGBA(const Characteristic space, const DATA *input, DATA *output, const int pixels) {
128+
static bool ConvertCbYCrAToRGBA(const Characteristic space, const DATA *input, DATA *output, const size_t pixels) {
125129
const float *matrix = GetYCbCrToRGBColorMatrix(space);
126130
if (matrix == NULL)
127131
return false;
128132
DATA RGBA[4];
129-
for (int i = 0; i < pixels; i++) {
133+
for (size_t i = 0; i < pixels; i++) {
130134
ConvertPixelYCbCrToRGB<DATA, max>(&input[i * 4], RGBA, matrix);
131135
RGBA[3] = input[i * 4 + 3];
132136
memcpy(&output[i * 4], RGBA, sizeof(DATA) * 4);
@@ -136,12 +140,12 @@ namespace dpx {
136140

137141
// 4:2:2
138142
template <typename DATA, unsigned int max>
139-
static bool ConvertCbYCrYToRGB(const Characteristic space, const DATA *input, DATA *output, const int pixels) {
143+
static bool ConvertCbYCrYToRGB(const Characteristic space, const DATA *input, DATA *output, const size_t pixels) {
140144
const float *matrix = GetYCbCrToRGBColorMatrix(space);
141145
if (matrix == NULL)
142146
return false;
143147
DATA CbYCr[3];
144-
for (int i = 0; i < pixels; i++) {
148+
for (size_t i = 0; i < pixels; i++) {
145149
// upsample to 4:4:4
146150
// FIXME: proper interpolation
147151
CbYCr[0] = input[(i | 1) * 2]; // Cb
@@ -155,12 +159,12 @@ namespace dpx {
155159

156160
// 4:2:2:4
157161
template <typename DATA, unsigned int max>
158-
static bool ConvertCbYACrYAToRGBA(const Characteristic space, const DATA *input, DATA *output, const int pixels) {
162+
static bool ConvertCbYACrYAToRGBA(const Characteristic space, const DATA *input, DATA *output, const size_t pixels) {
159163
const float *matrix = GetYCbCrToRGBColorMatrix(space);
160164
if (matrix == NULL)
161165
return false;
162166
DATA CbYCr[3];
163-
for (int i = 0; i < pixels; i++) {
167+
for (size_t i = 0; i < pixels; i++) {
164168
// upsample to 4:4:4
165169
// FIXME: proper interpolation
166170
CbYCr[0] = input[(i | 1) * 3]; // Cb
@@ -174,7 +178,7 @@ namespace dpx {
174178
}
175179

176180
static inline bool ConvertToRGBInternal(const Descriptor desc, const DataSize size, const Characteristic space,
177-
const void *input, void *output, const int pixels) {
181+
const void *input, void *output, const size_t pixels) {
178182
switch (desc) {
179183
// redundant calls
180184
case kRGB:
@@ -185,19 +189,19 @@ namespace dpx {
185189
case kABGR:
186190
switch (size) {
187191
case kByte:
188-
return SwapRGBABytes<U8>((const U8 *)input, (U8 *)output, pixels);
192+
return SwapRGBABytes(OIIO::cspan<U8>((const U8*)input, pixels*4), OIIO::span<U8>((U8*)output, pixels*4));
189193
case kWord:
190-
return SwapRGBABytes<U16>((const U16 *)input, (U16 *)output, pixels);
194+
return SwapRGBABytes(OIIO::cspan<U16>((const U16*)input, pixels*4), OIIO::span<U16>((U16*)output, pixels*4));
191195
case kInt:
192-
return SwapRGBABytes<U32>((const U32 *)input, (U32 *)output, pixels);
196+
return SwapRGBABytes(OIIO::cspan<U32>((const U32*)input, pixels*4), OIIO::span<U32>((U32*)output, pixels*4));
193197
case kFloat:
194-
return SwapRGBABytes<R32>((const R32 *)input, (R32 *)output, pixels);
198+
return SwapRGBABytes(OIIO::cspan<R32>((const R32*)input, pixels*4), OIIO::span<R32>((R32*)output, pixels*4));
195199
case kDouble:
196-
return SwapRGBABytes<R64>((const R64 *)input, (R64 *)output, pixels);
200+
return SwapRGBABytes(OIIO::cspan<R64>((const R64*)input, pixels*4), OIIO::span<R64>((R64*)output, pixels*4));
197201
}
198202
// shouldn't ever get here
199203
return false;
200-
204+
201205

202206
// FIXME: can this be translated to RGB?
203207
//case kCompositeVideo:
@@ -287,7 +291,7 @@ namespace dpx {
287291
}
288292
}
289293

290-
static inline int QueryRGBBufferSizeInternal(const Descriptor desc, const int pixels, const int bytes) {
294+
static inline int64_t QueryRGBBufferSizeInternal(const Descriptor desc, const size_t pixels, const size_t bytes) {
291295
switch (desc) {
292296
//case kCompositeVideo: // FIXME: can this be translated to RGB?
293297
case kCbYCrY: // 4:2:2 -> RGB, requires allocation
@@ -329,9 +333,10 @@ namespace dpx {
329333
}
330334
}
331335

332-
int QueryRGBBufferSize(const Header &header, const int element, const Block &block) {
336+
int64_t QueryRGBBufferSize(const Header &header, const size_t element, const Block &block) {
333337
return QueryRGBBufferSizeInternal(header.ImageDescriptor(element),
334-
(block.x2 - block.x1 + 1) * (block.y2 - block.y1 + 1),
338+
OIIO::clamped_mult64(block.x2 - block.x1 + 1,
339+
block.y2 - block.y1 + 1),
335340
header.ComponentByteCount(element));
336341
}
337342

@@ -344,10 +349,12 @@ namespace dpx {
344349
#endif /* NOT USED IN OIIO */
345350

346351
bool ConvertToRGB(const Header &header, const int element, const void *input, void *output, const Block &block) {
347-
return ConvertToRGBInternal(header.ImageDescriptor(element),
348-
header.ComponentDataSize(element), header.Colorimetric(element),
349-
input, output, (block.x2 - block.x1 + 1) * (block.y2 - block.y1 + 1));
350-
}
352+
return ConvertToRGBInternal(
353+
header.ImageDescriptor(element),
354+
header.ComponentDataSize(element), header.Colorimetric(element),
355+
input, output, OIIO::clamped_mult64(block.x2 - block.x1 + 1,
356+
block.y2 - block.y1 + 1));
357+
}
351358

352359
#if 0 /* NOT USED IN OIIO */
353360
bool ConvertToRGB(const Header &header, const int element, const void *input, void *output) {
@@ -479,7 +486,7 @@ namespace dpx {
479486
#endif /* NOT USED IN OIIO */
480487

481488
static inline bool ConvertToNativeInternal(const Descriptor desc, const DataSize size, const Characteristic space,
482-
const void *input, void *output, const int pixels) {
489+
const void *input, void *output, const size_t pixels) {
483490
switch (desc) {
484491
// redundant calls
485492
case kRGB:
@@ -491,15 +498,15 @@ namespace dpx {
491498
case kABGR:
492499
switch (size) {
493500
case kByte:
494-
return SwapRGBABytes<U8>((const U8 *)input, (U8 *)output, pixels);
501+
return SwapRGBABytes(OIIO::cspan<U8>((const U8*)input, pixels*4), OIIO::span<U8>((U8*)output, pixels*4));
495502
case kWord:
496-
return SwapRGBABytes<U16>((const U16 *)input, (U16 *)output, pixels);
503+
return SwapRGBABytes(OIIO::cspan<U16>((const U16*)input, pixels*4), OIIO::span<U16>((U16*)output, pixels*4));
497504
case kInt:
498-
return SwapRGBABytes<U32>((const U32 *)input, (U32 *)output, pixels);
505+
return SwapRGBABytes(OIIO::cspan<U32>((const U32*)input, pixels*4), OIIO::span<U32>((U32*)output, pixels*4));
499506
case kFloat:
500-
return SwapRGBABytes<R32>((const R32 *)input, (R32 *)output, pixels);
507+
return SwapRGBABytes(OIIO::cspan<R32>((const R32*)input, pixels*4), OIIO::span<R32>((R32*)output, pixels*4));
501508
case kDouble:
502-
return SwapRGBABytes<R64>((const R64 *)input, (R64 *)output, pixels);
509+
return SwapRGBABytes(OIIO::cspan<R64>((const R64*)input, pixels*4), OIIO::span<R64>((R64*)output, pixels*4));
503510
}
504511
// shouldn't ever get here
505512
return false;
@@ -647,7 +654,7 @@ namespace dpx {
647654
}
648655
#endif /* NOT USED IN OIIO */
649656

650-
bool ConvertToNative(const Descriptor desc, const DataSize compSize, const Characteristic cmetr, const int width, const int height, const void *input, void *output) {
651-
return ConvertToNativeInternal(desc, compSize, cmetr, input, output, width * height);
657+
bool ConvertToNative(const Descriptor desc, const DataSize compSize, const Characteristic cmetr, const size_t width, const size_t height, const void *input, void *output) {
658+
return ConvertToNativeInternal(desc, compSize, cmetr, input, output, OIIO::clamped_mult64(width, height));
652659
}
653660
}

‎src/dpx.imageio/libdpx/DPXColorConverter.h‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,9 @@ namespace dpx
5454
* of the buffer in bytes; sign: positive - memory needs to be allocated,
5555
* negative - allocation is optional, decoded data can replace the input
5656
*/
57-
int QueryRGBBufferSize(const Header &header, const int element, const Block &block);
57+
int64_t QueryRGBBufferSize(const Header &header, const size_t element, const Block &block);
5858

59+
#if 0 /* NOT USED IN OIIO */
5960
/*!
6061
* \brief Query the size of the buffer necessary to hold the decoded RGB data
6162
* \param header DPX header
@@ -64,7 +65,8 @@ namespace dpx
6465
* of the buffer in bytes; sign: positive - memory needs to be allocated,
6566
* negative - allocation is optional, decoded data can replace the input
6667
*/
67-
int QueryRGBBufferSize(const Header &header, const int element);
68+
int QueryRGBBufferSize(const Header &header, const size_t element);
69+
#endif
6870

6971
/*!
7072
* \brief Convert native data from the input buffer into RGB in the output buffer
@@ -78,6 +80,7 @@ namespace dpx
7880
*/
7981
bool ConvertToRGB(const Header &header, const int element, const void *input, void *output, const Block &block);
8082

83+
#if 0 /* NOT USED IN OIIO */
8184
/*!
8285
* \brief Convert native data from the input buffer into RGB in the output buffer
8386
* \param header DPX header
@@ -122,6 +125,7 @@ namespace dpx
122125
* \return success true/false
123126
*/
124127
bool ConvertToNative(const Descriptor desc, const DataSize compSize, const Characteristic cmetr, const void *input, void *output, const Block &block);
128+
#endif
125129

126130
/*!
127131
* \brief Convert RGB data from the input buffer into native format in the output buffer
@@ -132,7 +136,7 @@ namespace dpx
132136
* \param output output buffer data; can be same as input if \ref QueryNativeBufferSize returns a negative number
133137
* \return success true/false
134138
*/
135-
bool ConvertToNative(const Descriptor desc, const DataSize compSize, const Characteristic cmetr, const int width, const int height, const void *input, void *output);
139+
bool ConvertToNative(const Descriptor desc, const DataSize compSize, const Characteristic cmetr, const size_t width, const size_t height, const void *input, void *output);
136140

137141
}
138142

0 commit comments

Comments
 (0)