Skip to content

prevent signed int overflow in gdImageCopy functions#21173

Open
jordikroon wants to merge 5 commits into
php:PHP-8.4from
jordikroon:gh-21163
Open

prevent signed int overflow in gdImageCopy functions#21173
jordikroon wants to merge 5 commits into
php:PHP-8.4from
jordikroon:gh-21163

Conversation

@jordikroon
Copy link
Copy Markdown
Member

Fixes #21163; This affects all gdImageCopy.. functions.

@devnexen
Copy link
Copy Markdown
Member

devnexen commented Feb 8, 2026

You need to propose (first) the change upstream instead.

@devnexen
Copy link
Copy Markdown
Member

devnexen commented Feb 8, 2026

and you would need a new C test there.

@jordikroon
Copy link
Copy Markdown
Member Author

Upstream PR: libgd/libgd#982

Changes in here mirror the upstream.

@jordikroon
Copy link
Copy Markdown
Member Author

jordikroon commented May 26, 2026

This has been updated to reflect the changes in the upstream (libgd/libgd#997) that have been merged.

@devnexen could you give it another look?

@devnexen
Copy link
Copy Markdown
Member

devnexen commented May 26, 2026

Are you backporting a particular libgd commit or is it your PR content ? nevermind I was going to ask to wait Pierre Joye own fix and you did already :)

@jordikroon
Copy link
Copy Markdown
Member Author

jordikroon commented May 26, 2026

This is a backport (871c51c). Except the phpt file of course.

@devnexen
Copy link
Copy Markdown
Member

Nice only thing I realise you target master, I would not be surprised if the bug still apply in 8.4

@jordikroon jordikroon changed the base branch from master to PHP-8.4.0 May 26, 2026 19:45
@jordikroon jordikroon changed the base branch from PHP-8.4.0 to master May 26, 2026 19:46
@jordikroon jordikroon changed the base branch from master to PHP-8.4 May 26, 2026 19:50
@jordikroon
Copy link
Copy Markdown
Member Author

@devnexen alright, that's all sorted. It now targets PHP 8.4 as this also applies there.

Comment thread ext/gd/tests/gh21163.phpt
$expected = imagecolorat($dst, 0, 0);
imagecopyresampled($dst, $src, $nearIntMax, 0, 0, 0, $w, $h, 1, 1);
assertDstUnchanged('imagecopyresampled', $dst, $expected);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would like to see more edge cases as follows:

  • A negative dstX (say -5) with a small w/h that partially overlaps the destination. That way we confirm pixels from the right part of src end up in the left of dst,
    not just that nothing crashed.
    • A case where the source rectangle ends up empty after clipping, to make sure the resized variants don't end up dividing by zero internally.
    • One run against a palette image (imagecreate(...) instead of imagecreatetruecolor(...)) for imagecopy, since that goes through a different code path.
    • A very negative dstX (close to INT_MIN) as a mirror of the existing very-large-positive case.

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.

integer overflow in bundled gdImageCopy()

3 participants