Skip to content

Commit 33fd2e1

Browse files
committed
apps: 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.
1 parent 9921a85 commit 33fd2e1

File tree

5 files changed

+28
-20
lines changed

5 files changed

+28
-20
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ Version History
33

44
- Added Intel BMG-G31, Wildcat Lake, Nova Lake, and Crescent Island GPU support
55
- Added AMD RDNA 3.5 GPU support, extended RDNA 2 support
6+
- Fixed integer overflow and out-of-bounds write issues in image loaders (only affects the `oidnDenoise` example application)
67

78
### Changes in v2.3.3:
89

apps/utils/image_buffer.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ OIDN_NAMESPACE_BEGIN
1616
dataType(DataType::Void),
1717
format(Format::Undefined) {}
1818

19-
ImageBuffer::ImageBuffer(const DeviceRef& device, int width, int height, int numChannels,
19+
ImageBuffer::ImageBuffer(const DeviceRef& device, size_t width, size_t height, size_t numChannels,
2020
DataType dataType, Storage storage, bool forceHostCopy)
2121
: device(device),
2222
numValues(size_t(width) * height * numChannels),
@@ -26,6 +26,10 @@ OIDN_NAMESPACE_BEGIN
2626
dataType(dataType),
2727
format(makeFormat(dataType, numChannels))
2828
{
29+
30+
if (width > maxDim || height > maxDim || width * height * getC() > std::numeric_limits<int>::max())
31+
throw std::runtime_error("image size is too large");
32+
2933
const size_t valueByteSize = getDataTypeSize(dataType);
3034
byteSize = std::max(numValues * valueByteSize, size_t(1)); // avoid zero-sized buffer
3135
buffer = device.newBuffer(byteSize, storage);

apps/utils/image_buffer.h

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,19 @@ OIDN_NAMESPACE_BEGIN
1515
class ImageBuffer
1616
{
1717
public:
18+
static constexpr size_t maxDim = 65536;
19+
1820
ImageBuffer();
19-
ImageBuffer(const DeviceRef& device, int width, int height, int numChannels,
21+
ImageBuffer(const DeviceRef& device, size_t width, size_t height, size_t numChannels,
2022
DataType dataType = DataType::Float32,
2123
Storage storage = Storage::Undefined,
2224
bool forceHostCopy = false);
2325
~ImageBuffer();
2426

25-
oidn_inline int getW() const { return width; }
26-
oidn_inline int getH() const { return height; }
27-
oidn_inline int getC() const { return numChannels; }
28-
std::array<int, 3> getDims() const { return {width, height, numChannels}; }
27+
oidn_inline size_t getW() const { return width; }
28+
oidn_inline size_t getH() const { return height; }
29+
oidn_inline size_t getC() const { return numChannels; }
30+
std::array<size_t, 3> getDims() const { return {width, height, numChannels}; }
2931

3032
oidn_inline DataType getDataType() const { return dataType; }
3133
oidn_inline Format getFormat() const { return format; }
@@ -98,9 +100,9 @@ OIDN_NAMESPACE_BEGIN
98100
char* hostPtr;
99101
size_t byteSize;
100102
size_t numValues;
101-
int width;
102-
int height;
103-
int numChannels;
103+
size_t width;
104+
size_t height;
105+
size_t numChannels;
104106
DataType dataType;
105107
Format format;
106108
};

apps/utils/image_io.cpp

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ OIDN_NAMESPACE_BEGIN
6060
// Read the header
6161
std::string id;
6262
file >> id;
63-
int C;
63+
size_t C;
6464
if (id == "PF")
6565
C = 3;
6666
else if (id == "Pf")
@@ -73,7 +73,7 @@ OIDN_NAMESPACE_BEGIN
7373
if (dataType == DataType::Void)
7474
dataType = DataType::Float32;
7575

76-
int H, W;
76+
size_t H, W;
7777
file >> W >> H;
7878

7979
float scale;
@@ -91,23 +91,22 @@ OIDN_NAMESPACE_BEGIN
9191
// Read the pixels
9292
auto image = std::make_shared<ImageBuffer>(device, W, H, C, dataType, storage);
9393

94-
for (int h = 0; h < H; ++h)
94+
for (size_t h = 0; h < H; ++h)
9595
{
96-
for (int w = 0; w < W; ++w)
96+
for (size_t w = 0; w < W; ++w)
9797
{
98-
for (int c = 0; c < C; ++c)
98+
for (size_t c = 0; c < C; ++c)
9999
{
100100
float x;
101-
file.read((char*)&x, sizeof(float));
102-
if (c < C)
103-
image->set((size_t(H-1-h)*W + w) * C + c, x * scale);
101+
file.read(reinterpret_cast<char*>(&x), sizeof(float));
102+
if (file.fail()) {
103+
throw std::runtime_error("invalid PFM image: error reading pixel data");
104+
}
105+
image->set((size_t(H-1-h)*W + w) * C + c, x * scale);
104106
}
105107
}
106108
}
107109

108-
if (file.fail())
109-
throw std::runtime_error("invalid PFM image");
110-
111110
return image;
112111
}
113112

common/platform.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,8 @@ OIDN_NAMESPACE_BEGIN
265265
void* alignedMalloc(size_t size, size_t alignment = memoryAlignment);
266266
void alignedFree(void* ptr);
267267

268+
static_assert(sizeof(size_t) == 8, "size_t is not 64-bit!");
269+
268270
// -----------------------------------------------------------------------------------------------
269271
// String functions
270272
// -----------------------------------------------------------------------------------------------

0 commit comments

Comments
 (0)