Skip to content

fix integer overflow in _imaging.c#1711

Closed
nedwill wants to merge 1 commit into
python-pillow:masterfrom
nedwill:master
Closed

fix integer overflow in _imaging.c#1711
nedwill wants to merge 1 commit into
python-pillow:masterfrom
nedwill:master

Conversation

@nedwill

@nedwill nedwill commented Feb 4, 2016

Copy link
Copy Markdown

Fixes issue #1710. I added the canonical integer overflow check used across the CPython codebase.

Perhaps it's better to raise an OverflowError but I'm trying to keep some consistency with the surrounding code.

@wiredfool

Copy link
Copy Markdown
Member

Thanks for reporting this. I think we're going to include this in 3.1.1 today.

@wiredfool wiredfool added the Bug Any unexpected behavior, until confirmed feature. label Feb 4, 2016
@wiredfool wiredfool added this to the 3.1.1 milestone Feb 4, 2016
@wiredfool

Copy link
Copy Markdown
Member

I'm trying to add the test from the issue in a way that fails pre-patch, but I'm getting a failed malloc:

142     kk = malloc(xsize * kmax * sizeof(float));
(gdb) n
143     if ( ! kk)
(gdb) p kk
$1 = (float *) 0x0
(gdb) n
144         return (Imaging) ImagingError_MemoryError();

I've tried this on a small and large memory machine where it should have enough memory to allocate several Gb, but each time it winds up giving me a MemoryError. Unfortunately, this is indistinguishable from the post-patch case.

edit -- the test I'm using:

from PIL import Image
im = Image.open('Tests/images/hopper.png').convert('L')
xsize = 0x100000008 / 4
ysize = 1000 
im.im.resize((xsize, ysize), Image.LINEAR) 

wiredfool added a commit to wiredfool/Pillow that referenced this pull request Feb 4, 2016
wiredfool added a commit to wiredfool/Pillow that referenced this pull request Feb 4, 2016
wiredfool added a commit to wiredfool/Pillow that referenced this pull request Feb 4, 2016
wiredfool added a commit to wiredfool/Pillow that referenced this pull request Feb 4, 2016
@wiredfool

Copy link
Copy Markdown
Member

closed in favor of #1714

@wiredfool wiredfool closed this Feb 4, 2016
wiredfool added a commit that referenced this pull request Feb 4, 2016
@nedwill

nedwill commented Feb 4, 2016

Copy link
Copy Markdown
Author

@wiredfool it looks like you've already merged in the fix, but just to follow up: Because malloc takes a size_t as its argument, I think the overflow only occurs on 32-bit systems, where sizeof(size_t) == sizeof(int). You could raise OverflowError instead of MemoryError if you want to distinguish these cases.

@wiredfool

Copy link
Copy Markdown
Member

Makes sense, I'll check it out on a 32 bit vm

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

Labels

Bug Any unexpected behavior, until confirmed feature.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants