Bug #70052 [Csd]: getimagesize() fails for very large and very small WBMP

From: Date: Thu, 23 Jul 2015 22:01:28 +0000
Subject: Bug #70052 [Csd]: getimagesize() fails for very large and very small WBMP
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-194661@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=70052&edit=1

 ID:                 70052
 Updated by:         cmb@php.net
 Reported by:        p at wspnr dot com
 Summary:            getimagesize() fails for very large and very small
                     WBMP
 Status:             Closed
 Type:               Bug
 Package:            GetImageSize related
 Operating System:   Debian Linux
 PHP Version:        master-Git-2015-07-12 (Git)
 Assigned To:        cmb
 Block user comment: N
 Private report:     N

 New Comment:

Thanks for the notice. I've committed your patch now. Thanks!


Previous Comments:
------------------------------------------------------------------------
[2015-07-23 21:16:41] p at wspnr dot com

I submitted the doc patch a while ago, but I think it's still in the queue.

------------------------------------------------------------------------
[2015-07-23 16:46:30] cmb@php.net

Automatic comment on behalf of cmb
Revision: http://git.php.net/?p=php-src.git;a=commit;h=87829c09a1d9e39bee994460d7ccf19dd20eda14
Log: Fix #70052: getimagesize() fails for very large and very small WBMP

------------------------------------------------------------------------
[2015-07-13 12:40:56] p at wspnr dot com

Yep, I probably should have clarified that I was assuming the 2048x2048 limit stays. The width and
height probably should be converted to unsigned anyways as the WBMP format uses unsigned ints as
does the gfxinfo struct.

The 12-byte lower limit can be a problem for actual valid WBMPs, as it is a very compact format.

------------------------------------------------------------------------
[2015-07-13 12:18:54] cmb@php.net

> [...] I could probably submit a documentation patch [...]

That would be nice. :)

> However, I would still consider point #2 a bug.

ACK

> It should be possible to simply change the signed width and
> height to unsigned in php_get_wbmp().

AIUI that wouldn't help much, as the dimensions of WBMP can be
arbitrary integer values, so we'd only delay the overflow. If, on
the other hand, we'd stick with the size restriction (2048**2), we
could simply bail out early if the width (resp. height) exceeds
2048[1].

And it might be reasonable to also avoid the "Read error" for WBMP
with less than 12 bytes. If I'm not mistaken, a WBMP of 8x7 is
only 11 bytes large.

[1] <https://github.com/php/php-src/blob/php-5.6.11/ext/standard/image.c#L971>

------------------------------------------------------------------------
[2015-07-13 11:45:41] p at wspnr dot com

That sounds reasonable, I could probably submit a documentation patch unless something is already
being worked on.

However, I would still consider point #2 a bug. It should be possible to simply change the signed
width and height to unsigned in php_get_wbmp().

------------------------------------------------------------------------


The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at

    https://bugs.php.net/bug.php?id=70052


--
Edit this bug report at https://bugs.php.net/bug.php?id=70052&edit=1


Thread (11 messages)

« previous php.bugs (#194661) next »