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

Edit report at https://bugs.php.net/bug.php?id=70052&edit=1 ID: 70052 User updated by: p at wspnr dot com 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: I submitted the doc patch a while ago, but I think it's still in the queue. Previous Comments: ------------------------------------------------------------------------ [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(). ------------------------------------------------------------------------ [2015-07-13 08:45:47] cmb@php.net To my knowledge, MP4 files start with a 32bit number (big endian) that tells the size of the first atom. It *might* be possible that there are MP4 files which are mistaken for WBMP even though the dimensions are restricted for "valid" WBMP files. And of course it's always possible that there are some arbitrary binary files which can be mistaken for WBMP. It might be best to add a notice to the getimagesize() man page, that this function should not be used to try to detect whether a given file is an image file. finfo seems to be preferable for that purpose, but even that is not bullet proof. > Also, WBMPs smaller than 12 bytes produce a "Read error!". Indeed! That's caused by <https://github.com/php/php-src/blob/php-5.6.11/ext/standard/image.c#L1276-L1279>. ------------------------------------------------------------------------ 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