lvandeve / lodepng

PNG encoder and decoder in C and C++.
zlib License
2.07k stars 422 forks source link

Fixed segfault on large images #27

Closed evg-zhabotinsky closed 8 years ago

evg-zhabotinsky commented 8 years ago

Basically replaced all w * h with (unsigned long long) w * h.

w and h are unsigned so w * h overflows for large images. In fact, (w * h * bpp + 7) / 8 overflows for images that require buffer larger than 512MiB, which is not too much, for example 19200x10800 is just 10x ordinary screen resolution and with 24bpp it requires 593MiB.

Now it will only overflow on insanely large images, which would require too much memory anyway.

lvandeve commented 8 years ago

Hi,

unsigned long long is not part of C90, so not multiplatform enough.

Thanks for finding the crash, this is very helpful. I'll look into it later for a solution not using long longs.

ip commented 8 years ago

@evg-zhabotinsky Holy god :joy: Are such PNGs really used in practice?

lvandeve commented 8 years ago

The issue should now be fixed in the latest version

EDIT: That is, the segfault is fixed. It will not support the image, but return an error code.

evg-zhabotinsky commented 8 years ago

Holy god :joy: Are such PNGs really used in practice?

Well, I actually stumbled upon this crash while experimenting with raytracing. Just used 10x my monitor resolution for rendering. And I know that I am not the only one since there are couple SO questions about that. I just decided that it might be a good idea to finally report it.

Funny enough, even real displays are getting quite close. Four 8K UHD televisions on a wall are not quite a common sight yet, but not unreal, and their combined resolution is almost enough to cause that crash (actually enough if using alpha and more than 8 bits per channel).

lvandeve commented 8 years ago

It should support up to 268435455 pixels now, 16383x16383 pixels of 16-bit-per-channel RGBA.

When such images become more common in the future, 64-bit support for this can be added. For now not urgent I think though.