Hi,
I just wanted to plug https://bugs.php.net/bug.php?id=65873 , since
it's been a month since I filed it and I've only had silence in
response, despite sending a private email to Stas about it.
Sam Reed has reproduced this crash with two different PHP builds, with
no special configuration. CVE-2011-4566 (bug 60150) was from the same
line of code, which is kind of ridiculous. It'd be nice if someone
could have a closer look at the extension as a whole this time around,
maybe get rid of those offset_base pointers, rather than just doing a
one-line patch.
-- Tim Starling
Hi,
I just wanted to plug https://bugs.php.net/bug.php?id=65873 , since
it's been a month since I filed it and I've only had silence in
response, despite sending a private email to Stas about it.Sam Reed has reproduced this crash with two different PHP builds, with
no special configuration. CVE-2011-4566 (bug 60150) was from the same
line of code, which is kind of ridiculous. It'd be nice if someone
could have a closer look at the extension as a whole this time around,
maybe get rid of those offset_base pointers, rather than just doing a
one-line patch.
I think you just volunteered.
-Rasmus
Hi!
I just wanted to plug https://bugs.php.net/bug.php?id=65873 , since
it's been a month since I filed it and I've only had silence in
response, despite sending a private email to Stas about it.
Could you check out this patch: https://github.com/php/php-src/pull/539
It should fix this scenario. I couldn't add a test though since only
reproducing case is a 120M file and even for that special conditions are
required. If you have better reproduction that could be used on test
that would be most welcome.
Sam Reed has reproduced this crash with two different PHP builds, with
no special configuration. CVE-2011-4566 (bug 60150) was from the same
line of code, which is kind of ridiculous. It'd be nice if someone
could have a closer look at the extension as a whole this time around,
maybe get rid of those offset_base pointers, rather than just doing a
one-line patch.
I agree that offset_base, especially when it turns to negative
pseudo-pointer, looks strange, but fixing it goes beyond my
understanding of EXIF. Maybe Marcus (the extension maintainer) could
take a look at it.
--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227
Hi!
I just wanted to plug https://bugs.php.net/bug.php?id=65873 , since
it's been a month since I filed it and I've only had silence in
response, despite sending a private email to Stas about it.
Could you check out this patch: https://github.com/php/php-src/pull/539
It should fix this scenario.
I commented there.
I couldn't add a test though since only
reproducing case is a 120M file and even for that special conditions are
required. If you have better reproduction that could be used on test
that would be most welcome.
Well, reproduction requires that the file be bigger than the heap
pointer, so to reproduce reliably, you really need both a large file
and some control over the heap pointer. I think the best you could do
in a .phpt would be to use an ENV section to customise the allocator,
then craft a highly compressible TIFF file and gzinflate() it to a
temporary directory during test execution. But even that would be
system-dependent.
-- Tim Starling
Hi!
I just wanted to plug https://bugs.php.net/bug.php?id=65873 , since
it's been a month since I filed it and I've only had silence in
response, despite sending a private email to Stas about it.
Could you check out this patch: https://github.com/php/php-src/pull/539
It should fix this scenario.I commented there.
I couldn't add a test though since only
reproducing case is a 120M file and even for that special conditions are
required. If you have better reproduction that could be used on test
that would be most welcome.Well, reproduction requires that the file be bigger than the heap
pointer, so to reproduce reliably, you really need both a large file
and some control over the heap pointer. I think the best you could do
in a .phpt would be to use an ENV section to customise the allocator,
then craft a highly compressible TIFF file andgzinflate()it to a
temporary directory during test execution. But even that would be
system-dependent.
In any case, we can live without a test for this fix, better to have
it without test than no fix at all.
But generating it at runtime during the test run sounds like a good
solution. While I am not sure yet about a good way to craft a TIFF to
fit the crash requirements, we have no API to get the heap size (not
sure it is displayed by bintools).
Cheers,
Pierre
@pierrejoye | http://www.libgd.org
But generating it at runtime during the test run sounds like a good
solution. While I am not sure yet about a good way to craft a TIFF
to fit the crash requirements, we have no API to get the heap size
(not sure it is displayed by bintools).
Not the heap size, the heap pointer, i.e. (size_t)ptr, the distance
between the start of address space and the pointer returned by
emalloc(). On Linux, malloc() will sometimes use brk(), which gives
you an address after .text, .data, etc., so it is dependent on the
size of the main binary (which could be Apache, php-fcgi, etc.) --
typically it is some tens of megabytes above the start of the address
space.
If malloc() decides to use mmap() instead, on a 64-bit system, the
address returned will typically be terabytes above the start of
address space, so the attack will be infeasible. Windows apparently
doesn't have brk(), so the attack there would probably be limited to
32-bit systems.
-- Tim Starling