Skip to content

Conversation

@andr3y-k0v4l3v
Copy link
Contributor

  • Add checks for image dimensions (width/height/bits/channels) against ZEND_LONG_MAX.
  • Prevent sign-bit override when converting unsigned int to zend_long on 32-bit platforms.
  • Ensure consistent behavior across architectures for getimagesize() results.

Reported-by: Dmitriy Fedin [email protected]

@andr3y-k0v4l3v andr3y-k0v4l3v requested a review from bukka as a code owner May 26, 2025 12:18
@andr3y-k0v4l3v andr3y-k0v4l3v force-pushed the fix-unsafe-conversion-ext-standard-image branch 2 times, most recently from 2295fa9 to 0b1fae9 Compare May 26, 2025 14:47
Comment on lines +1524 to +1530
if (result &&
result->width <= INT32_MAX &&
result->height <= INT32_MAX &&
result->bits <= INT32_MAX &&
result->channels <= INT32_MAX)
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can check whether it's a 32-bit build and use conditional compilation accordingly.

Suggested change
if (result &&
result->width <= INT32_MAX &&
result->height <= INT32_MAX &&
result->bits <= INT32_MAX &&
result->channels <= INT32_MAX)
{
#if SIZEOF_ZEND_LONG == 4
if (result &&
result->width <= INT32_MAX &&
result->height <= INT32_MAX &&
result->bits <= INT32_MAX &&
result->channels <= INT32_MAX)
{
#else
if (result) {
#endif

- Add checks for image dimensions (width/height/bits/channels) against
INT32_MAX.
- Prevent sign-bit override when converting unsigned int to zend_long on 32-bit
platforms.
- Ensure consistent behavior across architectures for getimagesize() results.

Reported-by: Dmitriy Fedin <[email protected]>
Signed-off-by: Andrey Kovalev <[email protected]>
@andr3y-k0v4l3v andr3y-k0v4l3v force-pushed the fix-unsafe-conversion-ext-standard-image branch from 0b1fae9 to 3f92617 Compare June 2, 2025 07:37
@nielsdos
Copy link
Member

nielsdos commented Jun 2, 2025

Why not just update the format string?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants