Skip to content

Avoid integer overflow when calculating reduced image size - #9904

Open
lazerg wants to merge 3 commits into
python-pillow:mainfrom
lazerg:fix/issue-9903-reduce-overflow
Open

Avoid integer overflow when calculating reduced image size#9904
lazerg wants to merge 3 commits into
python-pillow:mainfrom
lazerg:fix/issue-9903-reduce-overflow

Conversation

@lazerg

@lazerg lazerg commented Aug 25, 2026

Copy link
Copy Markdown

ImagingReduce() sizes the output image with (box[2] + xscale - 1) / xscale. When the box width plus the scale exceeds INT_MAX that addition overflows and the result is a zero-width, zero-height image. ImagingReduceCorners() then writes to imOut->image8[0][0], which has no rows allocated, so Image.new("L", (4, 4)).reduce(2**31 - 1) segfaults.

Since the box is never empty, the same round-up can be written as (box[2] - 1) / xscale + 1, which cannot overflow and gives a 1x1 image as expected.

Fixes #9903

Comment thread Tests/test_image_reduce.py Outdated
Comment thread src/libImaging/Reduce.c

imOut = ImagingNewDirty(
imIn->mode, (box[2] + xscale - 1) / xscale, (box[3] + yscale - 1) / yscale
imIn->mode, (box[2] - 1) / xscale + 1, (box[3] - 1) / yscale + 1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd maybe add a comment here about the operation order, and that it guarantees the image to have at least size 1x1.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Added in bf60f3c.

While checking this I noticed that xscale * yscale in ImagingReduceNxN and (box[2] % xscale) * yscale in ImagingReduceCorners still overflow with a factor this large. Those values only feed loops that run zero iterations, so the output pixels are correct and no bad memory access happens, but an instrumented build will still report the overflow on the same input. Clamping both scales to the box size after the size calculation would remove it.

I am happy to add that here, or to keep this PR to the crash and raise it separately.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What do you mean with "Those values only feed loops that run zero iterations"? Those functions probably shouldn't end up being called...

In fact, it could be a good idea to add separate paths when the reduction ends up being one-dimensional in one dimension or the other, or both, since isn't the end result then just an average over rows, columns or both (separable)?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Image.reduce() segfaults when passed 2**31 - 1 as the reduction factor

3 participants