From bc7c56e05d414055f272172bbd484ca20f9834e0 Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Mon, 27 Apr 2026 14:04:22 -0700 Subject: [PATCH 1/2] 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 --- src/dpx.imageio/dpxinput.cpp | 5 +- src/dpx.imageio/libdpx/DPXColorConverter.cpp | 85 +++++++++++--------- src/dpx.imageio/libdpx/DPXColorConverter.h | 10 ++- 3 files changed, 57 insertions(+), 43 deletions(-) diff --git a/src/dpx.imageio/dpxinput.cpp b/src/dpx.imageio/dpxinput.cpp index 6675e95ffe..4f198046aa 100644 --- a/src/dpx.imageio/dpxinput.cpp +++ b/src/dpx.imageio/dpxinput.cpp @@ -208,6 +208,8 @@ DPXInput::seek_subimage(int subimage, int miplevel) m_spec = ImageSpec(m_dpx.header.Width(), m_dpx.header.Height(), m_dpx.header.ImageElementComponentCount(subimage), typedesc); + if (!check_open(m_spec, { 0, 1 << 20, 0, 1 << 20, 0, 1 << 16, 0, 8 })) + return false; // xOffset/yOffset are defined as unsigned 32-bit integers, but m_spec.x/y are signed // avoid casts that would result in negative values @@ -599,7 +601,8 @@ DPXInput::read_native_scanlines(int subimage, int miplevel, int ybegin, } else { // read the scanline and convert to RGB unsigned char* ptr = (unsigned char*)data; - int bufsize = dpx::QueryRGBBufferSize(m_dpx.header, subimage, block); + int64_t bufsize = dpx::QueryRGBBufferSize(m_dpx.header, subimage, + block); if (bufsize > 0) { m_decodebuf.resize(bufsize); ptr = m_decodebuf.data(); diff --git a/src/dpx.imageio/libdpx/DPXColorConverter.cpp b/src/dpx.imageio/libdpx/DPXColorConverter.cpp index 0fc9701abe..48ee85f760 100644 --- a/src/dpx.imageio/libdpx/DPXColorConverter.cpp +++ b/src/dpx.imageio/libdpx/DPXColorConverter.cpp @@ -34,19 +34,23 @@ #include "DPXColorConverter.h" +#include +#include #include namespace dpx { template - static inline bool SwapRGBABytes(const DATA *input, DATA *output, int pixels) { - // copy the data that could be destroyed to an additional buffer in case input == output - DATA tmp[2]; - for (int i = 0; i < pixels; i++) { - memcpy(tmp, &input[i * 4], sizeof(DATA) * 2); - output[i * 4 + 0] = input[i * 4 + 3]; - output[i * 4 + 1] = input[i * 4 + 2]; - output[i * 4 + 2] = tmp[1]; - output[i * 4 + 3] = tmp[0]; + static inline bool SwapRGBABytes(OIIO::cspan input, OIIO::span output) { + size_t pixels = input.size() / 4; + if (pixels * 4 + 3 >= input.size() || pixels * 4 + 3 >= output.size()) + return false; + // Because we checked the lengths, we can just use data pointers below + for (size_t i = 0; i < pixels; i++) { + DATA a = input.data()[i * 4 + 0], b = input.data()[i * 4 + 1]; + output.data()[i * 4 + 0] = input.data()[i * 4 + 3]; + output.data()[i * 4 + 1] = input.data()[i * 4 + 2]; + output.data()[i * 4 + 2] = b; + output.data()[i * 4 + 3] = a; } return true; } @@ -107,12 +111,12 @@ namespace dpx { // 4:4:4 template - static bool ConvertCbYCrToRGB(const Characteristic space, const DATA *input, DATA *output, const int pixels) { + static bool ConvertCbYCrToRGB(const Characteristic space, const DATA *input, DATA *output, const size_t pixels) { const float *matrix = GetYCbCrToRGBColorMatrix(space); if (matrix == NULL) return false; DATA RGB[3]; - for (int i = 0; i < pixels; i++) { + for (size_t i = 0; i < pixels; i++) { ConvertPixelYCbCrToRGB(&input[i * 3], RGB, matrix); memcpy(&output[i * 3], RGB, sizeof(DATA) * 3); } @@ -121,12 +125,12 @@ namespace dpx { // 4:4:4:4 template - static bool ConvertCbYCrAToRGBA(const Characteristic space, const DATA *input, DATA *output, const int pixels) { + static bool ConvertCbYCrAToRGBA(const Characteristic space, const DATA *input, DATA *output, const size_t pixels) { const float *matrix = GetYCbCrToRGBColorMatrix(space); if (matrix == NULL) return false; DATA RGBA[4]; - for (int i = 0; i < pixels; i++) { + for (size_t i = 0; i < pixels; i++) { ConvertPixelYCbCrToRGB(&input[i * 4], RGBA, matrix); RGBA[3] = input[i * 4 + 3]; memcpy(&output[i * 4], RGBA, sizeof(DATA) * 4); @@ -136,12 +140,12 @@ namespace dpx { // 4:2:2 template - static bool ConvertCbYCrYToRGB(const Characteristic space, const DATA *input, DATA *output, const int pixels) { + static bool ConvertCbYCrYToRGB(const Characteristic space, const DATA *input, DATA *output, const size_t pixels) { const float *matrix = GetYCbCrToRGBColorMatrix(space); if (matrix == NULL) return false; DATA CbYCr[3]; - for (int i = 0; i < pixels; i++) { + for (size_t i = 0; i < pixels; i++) { // upsample to 4:4:4 // FIXME: proper interpolation CbYCr[0] = input[(i | 1) * 2]; // Cb @@ -155,12 +159,12 @@ namespace dpx { // 4:2:2:4 template - static bool ConvertCbYACrYAToRGBA(const Characteristic space, const DATA *input, DATA *output, const int pixels) { + static bool ConvertCbYACrYAToRGBA(const Characteristic space, const DATA *input, DATA *output, const size_t pixels) { const float *matrix = GetYCbCrToRGBColorMatrix(space); if (matrix == NULL) return false; DATA CbYCr[3]; - for (int i = 0; i < pixels; i++) { + for (size_t i = 0; i < pixels; i++) { // upsample to 4:4:4 // FIXME: proper interpolation CbYCr[0] = input[(i | 1) * 3]; // Cb @@ -174,7 +178,7 @@ namespace dpx { } static inline bool ConvertToRGBInternal(const Descriptor desc, const DataSize size, const Characteristic space, - const void *input, void *output, const int pixels) { + const void *input, void *output, const size_t pixels) { switch (desc) { // redundant calls case kRGB: @@ -185,19 +189,19 @@ namespace dpx { case kABGR: switch (size) { case kByte: - return SwapRGBABytes((const U8 *)input, (U8 *)output, pixels); + return SwapRGBABytes(OIIO::cspan((const U8*)input, pixels*4), OIIO::span((U8*)output, pixels*4)); case kWord: - return SwapRGBABytes((const U16 *)input, (U16 *)output, pixels); + return SwapRGBABytes(OIIO::cspan((const U16*)input, pixels*4), OIIO::span((U16*)output, pixels*4)); case kInt: - return SwapRGBABytes((const U32 *)input, (U32 *)output, pixels); + return SwapRGBABytes(OIIO::cspan((const U32*)input, pixels*4), OIIO::span((U32*)output, pixels*4)); case kFloat: - return SwapRGBABytes((const R32 *)input, (R32 *)output, pixels); + return SwapRGBABytes(OIIO::cspan((const R32*)input, pixels*4), OIIO::span((R32*)output, pixels*4)); case kDouble: - return SwapRGBABytes((const R64 *)input, (R64 *)output, pixels); + return SwapRGBABytes(OIIO::cspan((const R64*)input, pixels*4), OIIO::span((R64*)output, pixels*4)); } // shouldn't ever get here return false; - + // FIXME: can this be translated to RGB? //case kCompositeVideo: @@ -287,7 +291,7 @@ namespace dpx { } } - static inline int QueryRGBBufferSizeInternal(const Descriptor desc, const int pixels, const int bytes) { + static inline int64_t QueryRGBBufferSizeInternal(const Descriptor desc, const size_t pixels, const size_t bytes) { switch (desc) { //case kCompositeVideo: // FIXME: can this be translated to RGB? case kCbYCrY: // 4:2:2 -> RGB, requires allocation @@ -329,9 +333,10 @@ namespace dpx { } } - int QueryRGBBufferSize(const Header &header, const int element, const Block &block) { + int64_t QueryRGBBufferSize(const Header &header, const size_t element, const Block &block) { return QueryRGBBufferSizeInternal(header.ImageDescriptor(element), - (block.x2 - block.x1 + 1) * (block.y2 - block.y1 + 1), + OIIO::clamped_mult64(block.x2 - block.x1 + 1, + block.y2 - block.y1 + 1), header.ComponentByteCount(element)); } @@ -344,10 +349,12 @@ namespace dpx { #endif /* NOT USED IN OIIO */ bool ConvertToRGB(const Header &header, const int element, const void *input, void *output, const Block &block) { - return ConvertToRGBInternal(header.ImageDescriptor(element), - header.ComponentDataSize(element), header.Colorimetric(element), - input, output, (block.x2 - block.x1 + 1) * (block.y2 - block.y1 + 1)); - } + return ConvertToRGBInternal( + header.ImageDescriptor(element), + header.ComponentDataSize(element), header.Colorimetric(element), + input, output, OIIO::clamped_mult64(block.x2 - block.x1 + 1, + block.y2 - block.y1 + 1)); + } #if 0 /* NOT USED IN OIIO */ bool ConvertToRGB(const Header &header, const int element, const void *input, void *output) { @@ -479,7 +486,7 @@ namespace dpx { #endif /* NOT USED IN OIIO */ static inline bool ConvertToNativeInternal(const Descriptor desc, const DataSize size, const Characteristic space, - const void *input, void *output, const int pixels) { + const void *input, void *output, const size_t pixels) { switch (desc) { // redundant calls case kRGB: @@ -491,15 +498,15 @@ namespace dpx { case kABGR: switch (size) { case kByte: - return SwapRGBABytes((const U8 *)input, (U8 *)output, pixels); + return SwapRGBABytes(OIIO::cspan((const U8*)input, pixels*4), OIIO::span((U8*)output, pixels*4)); case kWord: - return SwapRGBABytes((const U16 *)input, (U16 *)output, pixels); + return SwapRGBABytes(OIIO::cspan((const U16*)input, pixels*4), OIIO::span((U16*)output, pixels*4)); case kInt: - return SwapRGBABytes((const U32 *)input, (U32 *)output, pixels); + return SwapRGBABytes(OIIO::cspan((const U32*)input, pixels*4), OIIO::span((U32*)output, pixels*4)); case kFloat: - return SwapRGBABytes((const R32 *)input, (R32 *)output, pixels); + return SwapRGBABytes(OIIO::cspan((const R32*)input, pixels*4), OIIO::span((R32*)output, pixels*4)); case kDouble: - return SwapRGBABytes((const R64 *)input, (R64 *)output, pixels); + return SwapRGBABytes(OIIO::cspan((const R64*)input, pixels*4), OIIO::span((R64*)output, pixels*4)); } // shouldn't ever get here return false; @@ -647,7 +654,7 @@ namespace dpx { } #endif /* NOT USED IN OIIO */ - bool ConvertToNative(const Descriptor desc, const DataSize compSize, const Characteristic cmetr, const int width, const int height, const void *input, void *output) { - return ConvertToNativeInternal(desc, compSize, cmetr, input, output, width * height); + bool ConvertToNative(const Descriptor desc, const DataSize compSize, const Characteristic cmetr, const size_t width, const size_t height, const void *input, void *output) { + return ConvertToNativeInternal(desc, compSize, cmetr, input, output, OIIO::clamped_mult64(width, height)); } } diff --git a/src/dpx.imageio/libdpx/DPXColorConverter.h b/src/dpx.imageio/libdpx/DPXColorConverter.h index 7940992aaf..7a3828ffad 100644 --- a/src/dpx.imageio/libdpx/DPXColorConverter.h +++ b/src/dpx.imageio/libdpx/DPXColorConverter.h @@ -54,8 +54,9 @@ namespace dpx * of the buffer in bytes; sign: positive - memory needs to be allocated, * negative - allocation is optional, decoded data can replace the input */ - int QueryRGBBufferSize(const Header &header, const int element, const Block &block); + int64_t QueryRGBBufferSize(const Header &header, const size_t element, const Block &block); +#if 0 /* NOT USED IN OIIO */ /*! * \brief Query the size of the buffer necessary to hold the decoded RGB data * \param header DPX header @@ -64,7 +65,8 @@ namespace dpx * of the buffer in bytes; sign: positive - memory needs to be allocated, * negative - allocation is optional, decoded data can replace the input */ - int QueryRGBBufferSize(const Header &header, const int element); + int QueryRGBBufferSize(const Header &header, const size_t element); +#endif /*! * \brief Convert native data from the input buffer into RGB in the output buffer @@ -78,6 +80,7 @@ namespace dpx */ bool ConvertToRGB(const Header &header, const int element, const void *input, void *output, const Block &block); +#if 0 /* NOT USED IN OIIO */ /*! * \brief Convert native data from the input buffer into RGB in the output buffer * \param header DPX header @@ -122,6 +125,7 @@ namespace dpx * \return success true/false */ bool ConvertToNative(const Descriptor desc, const DataSize compSize, const Characteristic cmetr, const void *input, void *output, const Block &block); +#endif /*! * \brief Convert RGB data from the input buffer into native format in the output buffer @@ -132,7 +136,7 @@ namespace dpx * \param output output buffer data; can be same as input if \ref QueryNativeBufferSize returns a negative number * \return success true/false */ - bool ConvertToNative(const Descriptor desc, const DataSize compSize, const Characteristic cmetr, const int width, const int height, const void *input, void *output); + bool ConvertToNative(const Descriptor desc, const DataSize compSize, const Characteristic cmetr, const size_t width, const size_t height, const void *input, void *output); } From bbebd3b634e7028d89bcc2162a4b9cfdf0eb5e91 Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Tue, 28 Apr 2026 15:14:09 -0700 Subject: [PATCH 2/2] Remove useless test from SwapRGBABytes Signed-off-by: Larry Gritz --- src/dpx.imageio/libdpx/DPXColorConverter.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/dpx.imageio/libdpx/DPXColorConverter.cpp b/src/dpx.imageio/libdpx/DPXColorConverter.cpp index 48ee85f760..18ab4ea12a 100644 --- a/src/dpx.imageio/libdpx/DPXColorConverter.cpp +++ b/src/dpx.imageio/libdpx/DPXColorConverter.cpp @@ -41,10 +41,9 @@ namespace dpx { template static inline bool SwapRGBABytes(OIIO::cspan input, OIIO::span output) { - size_t pixels = input.size() / 4; - if (pixels * 4 + 3 >= input.size() || pixels * 4 + 3 >= output.size()) - return false; - // Because we checked the lengths, we can just use data pointers below + // Because we are within the lengths by definition, we can just use + // data pointers below to avoid a bounds check on every one. + size_t pixels = input.size() / 4; for (size_t i = 0; i < pixels; i++) { DATA a = input.data()[i * 4 + 0], b = input.data()[i * 4 + 1]; output.data()[i * 4 + 0] = input.data()[i * 4 + 3];