-
Notifications
You must be signed in to change notification settings - Fork 185
apps: fix integer-overflow and OOB write issues in image loaders #280
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR hardens image loading code against malicious or malformed inputs by addressing integer overflow and out-of-bounds write vulnerabilities. The changes focus on preventing heap-based exploits that could occur when processing untrusted image files, particularly PFM format files.
Key changes:
- Switches dimension and channel types from
inttosize_tto prevent signed integer overflow - Adds validation to reject excessively large images (maxDim=64K) and ensures total pixel counts don't exceed
intlimits - Improves PFM loader error handling with early validation of pixel read operations
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| common/platform.h | Adds compile-time assertion ensuring size_t is 64-bit on all supported platforms |
| apps/utils/image_io.cpp | Changes PFM loader to use size_t for dimensions/channels and adds per-pixel read validation |
| apps/utils/image_buffer.h | Updates ImageBuffer interface to use size_t for dimensions and adds maxDim constant |
| apps/utils/image_buffer.cpp | Implements validation checks to prevent overflow during buffer allocation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
atafra
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Just a few more minor things:
- Please update commit message to: "apps: fix integer-overflow..."
- Please update changelog, without "apps:" prefix but mentioning that only
oidnDenoiseis affected.
465b46e to
03e7afe
Compare
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.
03e7afe to
33fd2e1
Compare
atafra
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ready to merge!
This patch hardens ImageBuffer and the PFM loader against malformed or malicious inputs:
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.