From 465b46ec748a9c4bc918196519df12569fde405e Mon Sep 17 00:00:00 2001 From: Krzysztof Raszkowski Date: Fri, 21 Nov 2025 11:01:42 -0600 Subject: [PATCH 1/2] Fix integer-overflow and OOB write issues in image loaders This patch hardens ImageBuffer and the PFM loader against malformed or malicious inputs: - Switch width/height/channels from int to size_t to avoid signed overflow. - Add maxDim guard (64K) and reject images whose total pixel count exceeds int limits to prevent integer multiplication overflow. - Validate pixel reads during PFM parsing and fail early on read errors. - Use overflow-safe width*height*C checks before allocating the buffer. - Add static_assert ensuring size_t is 64-bit on supported platforms. These changes prevent tiny allocations followed by large writes caused by untrusted PFM headers (e.g., malicious W/H/C), removing the possibility of heap-based out-of-bounds writes. --- apps/utils/image_buffer.cpp | 6 +++++- apps/utils/image_buffer.h | 18 ++++++++++-------- apps/utils/image_io.cpp | 21 ++++++++++----------- common/platform.h | 2 ++ 4 files changed, 27 insertions(+), 20 deletions(-) diff --git a/apps/utils/image_buffer.cpp b/apps/utils/image_buffer.cpp index e9a6e607..0bf454a5 100644 --- a/apps/utils/image_buffer.cpp +++ b/apps/utils/image_buffer.cpp @@ -16,7 +16,7 @@ OIDN_NAMESPACE_BEGIN dataType(DataType::Void), format(Format::Undefined) {} - ImageBuffer::ImageBuffer(const DeviceRef& device, int width, int height, int numChannels, + ImageBuffer::ImageBuffer(const DeviceRef& device, size_t width, size_t height, size_t numChannels, DataType dataType, Storage storage, bool forceHostCopy) : device(device), numValues(size_t(width) * height * numChannels), @@ -26,6 +26,10 @@ OIDN_NAMESPACE_BEGIN dataType(dataType), format(makeFormat(dataType, numChannels)) { + + if (width > maxDim || height > maxDim || width * height * getC() > std::numeric_limits::max()) + throw std::runtime_error("image size is too large"); + const size_t valueByteSize = getDataTypeSize(dataType); byteSize = std::max(numValues * valueByteSize, size_t(1)); // avoid zero-sized buffer buffer = device.newBuffer(byteSize, storage); diff --git a/apps/utils/image_buffer.h b/apps/utils/image_buffer.h index a262aae9..66777f6b 100644 --- a/apps/utils/image_buffer.h +++ b/apps/utils/image_buffer.h @@ -15,17 +15,19 @@ OIDN_NAMESPACE_BEGIN class ImageBuffer { public: + static constexpr size_t maxDim = 65536; + ImageBuffer(); - ImageBuffer(const DeviceRef& device, int width, int height, int numChannels, + ImageBuffer(const DeviceRef& device, size_t width, size_t height, size_t numChannels, DataType dataType = DataType::Float32, Storage storage = Storage::Undefined, bool forceHostCopy = false); ~ImageBuffer(); - oidn_inline int getW() const { return width; } - oidn_inline int getH() const { return height; } - oidn_inline int getC() const { return numChannels; } - std::array getDims() const { return {width, height, numChannels}; } + oidn_inline size_t getW() const { return width; } + oidn_inline size_t getH() const { return height; } + oidn_inline size_t getC() const { return numChannels; } + std::array getDims() const { return {width, height, numChannels}; } oidn_inline DataType getDataType() const { return dataType; } oidn_inline Format getFormat() const { return format; } @@ -98,9 +100,9 @@ OIDN_NAMESPACE_BEGIN char* hostPtr; size_t byteSize; size_t numValues; - int width; - int height; - int numChannels; + size_t width; + size_t height; + size_t numChannels; DataType dataType; Format format; }; diff --git a/apps/utils/image_io.cpp b/apps/utils/image_io.cpp index e772e469..ace7d3ca 100644 --- a/apps/utils/image_io.cpp +++ b/apps/utils/image_io.cpp @@ -60,7 +60,7 @@ OIDN_NAMESPACE_BEGIN // Read the header std::string id; file >> id; - int C; + size_t C; if (id == "PF") C = 3; else if (id == "Pf") @@ -73,7 +73,7 @@ OIDN_NAMESPACE_BEGIN if (dataType == DataType::Void) dataType = DataType::Float32; - int H, W; + size_t H, W; file >> W >> H; float scale; @@ -91,23 +91,22 @@ OIDN_NAMESPACE_BEGIN // Read the pixels auto image = std::make_shared(device, W, H, C, dataType, storage); - for (int h = 0; h < H; ++h) + for (size_t h = 0; h < H; ++h) { - for (int w = 0; w < W; ++w) + for (size_t w = 0; w < W; ++w) { - for (int c = 0; c < C; ++c) + for (size_t c = 0; c < C; ++c) { float x; - file.read((char*)&x, sizeof(float)); - if (c < C) - image->set((size_t(H-1-h)*W + w) * C + c, x * scale); + file.read(reinterpret_cast(&x), sizeof(float)); + if (file.fail()) { + throw std::runtime_error("invalid PFM image: error reading pixel data"); + } + image->set((size_t(H-1-h)*W + w) * C + c, x * scale); } } } - if (file.fail()) - throw std::runtime_error("invalid PFM image"); - return image; } diff --git a/common/platform.h b/common/platform.h index 04b9370c..8b99338f 100644 --- a/common/platform.h +++ b/common/platform.h @@ -265,6 +265,8 @@ OIDN_NAMESPACE_BEGIN void* alignedMalloc(size_t size, size_t alignment = memoryAlignment); void alignedFree(void* ptr); + static_assert(sizeof(size_t) == 8, "size_t is not 64-bit!"); + // ----------------------------------------------------------------------------------------------- // String functions // ----------------------------------------------------------------------------------------------- From 2013c9263bb7f9ae32ab569abde56b59d9f37171 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 08:31:21 +0000 Subject: [PATCH 2/2] Initial plan