hi everyone,
i'm looking for a sanity check here, as i've already lost more time than
i'd like chasing ghosts on my treasure hunt through {bugs,lists,cvs}.php.net :(
afaict, CVE-2008-5658[1] is only half-fixed on 5.2.8, while it was supposed
to be fixed in 5.2.7.
while the zip library no longer blindly extracts files such as
"../../../var/www/index.php", it now seems to segfault on any files
that have a leading "..". I've put some sample code illustrating my
problem at[2]. am i on crack?
a backtrace points to virtual_file_ex() returning an unchecked error in
php_zip_extract_file(). it looks like there might have been a fix in the 5.3
branch, but it was surrounded by so much other noise that i'm not sure.
i guess someone here knows better than me what's going on. it doesn't seem
exploitable for more than a DoS at first glance, but i'll defer to the experts
on that as well.
sean
[1] http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2008-5658
[2] http://people.debian.org/~seanius/php/security/ziptest.tgz
hi,
hi everyone,
i'm looking for a sanity check here, as i've already lost more time than
i'd like chasing ghosts on my treasure hunt through {bugs,lists,cvs}.php.net :(afaict, CVE-2008-5658[1] is only half-fixed on 5.2.8, while it was supposed
to be fixed in 5.2.7.
it is fixed in 5.2.7RC2 or RC3, see:
http://cvs.php.net/viewvc.cgi/php-src/ext/zip/php_zip.c?r1=1.1.2.43&r2=1.1.2.44
while the zip library no longer blindly extracts files such as
"../../../var/www/index.php", it now seems to segfault on any files
that have a leading "..". I've put some sample code illustrating my
problem at[2]. am i on crack?
No idea, can you open a bug and post the backtrace, a zip data to
reproduce the problem and a simple script please? Simply post the
links you gave here. I will take a look at them as soon as possible.
Thanks for the report!
Cheers,
Pierre
re,
I ran a quick test to solve this problem sooner rather than later
(using only the crash.zip part):
pierre@ubuntu:~/cvs/php53/bld$ ./sapi/cli/php ./ziptest.php
opening 'bad' zipfile...ok.
extracted.
C:\Users\pierre\Documents\php-sdk\php53\vc9\x86\php53clean>Debug\php.exe
ziptest.php
opening 'bad' zipfile...ok.
extracted.
But it crashes in 5.2, it seems to be a problem in virtual_file_ex, it
return an empty string instead of the expected path.
Can you try the attached patch please? against 5.2. I backported the
necessary functions from TSRM and removed what we do not use. It
should fix the problem.
Cheers,
hi,
hi everyone,
i'm looking for a sanity check here, as i've already lost more time than
i'd like chasing ghosts on my treasure hunt through {bugs,lists,cvs}.php.net :(afaict, CVE-2008-5658[1] is only half-fixed on 5.2.8, while it was supposed
to be fixed in 5.2.7.it is fixed in 5.2.7RC2 or RC3, see:
http://cvs.php.net/viewvc.cgi/php-src/ext/zip/php_zip.c?r1=1.1.2.43&r2=1.1.2.44while the zip library no longer blindly extracts files such as
"../../../var/www/index.php", it now seems to segfault on any files
that have a leading "..". I've put some sample code illustrating my
problem at[2]. am i on crack?No idea, can you open a bug and post the backtrace, a zip data to
reproduce the problem and a simple script please? Simply post the
links you gave here. I will take a look at them as soon as possible.Thanks for the report!
Cheers,
Pierre
--
Pierre
hi pierre,
i've tested the patch that you proposed via IRC (attached) and it seems
to work for me against 5.2.8. passes valgrind too, without any detected
errors or leaks.
it's unfortunate that there isn't a more surgical fix (301 insertions!),
but i'll take your word for it that it would be too complicated/dangerous
to try and modify virtual_file_ex() directly.
thanks!
sean
hi again,
it's unfortunate that there isn't a more surgical fix (301 insertions!),
but i'll take your word for it that it would be too complicated/dangerous
to try and modify virtual_file_ex() directly.
actually, i think i've found a slightly more graceful workaround :)
since virtual_file_ex is to fragile to be changed, here's a patch that
does the following as a workaround:
- take a temporary copy of the filename
- replace all instances of "^../", "/../", and "/..$" with "///".
- pass this mangled filename to virtual_file_ex for normalization
it seems virtual_file_ex can handle such a filename without problem, and
with proper formatting the current patch only inserts 22 lines to php_zip.c.
someone should probably double check this code for early-morning coding
errors though :)
what do you think?
sean
hi pierre
sorry, was already asleep when you came looking for me on IRC :)
it is fixed in 5.2.7RC2 or RC3, see:
http://cvs.php.net/viewvc.cgi/php-src/ext/zip/php_zip.c?r1=1.1.2.43&r2=1.1.2.44
FSVO "fixed" that includes segfaulting, anyway :)
No idea, can you open a bug and post the backtrace, a zip data to
reproduce the problem and a simple script please? Simply post the
links you gave here. I will take a look at them as soon as possible.
But it crashes in 5.2, it seems to be a problem in virtual_file_ex, it
return an empty string instead of the expected path.
/* Resolve path relatively to state and put the real path into state /
/ returns 0 for ok, 1 for error */
and it's returning 1 in this case, so it's an unhandled error, which is
then also unhandled in php_zip_extract_file, as previously suggested.
Can you try the attached patch please? against 5.2. I backported the
necessary functions from TSRM and removed what we do not use. It
should fix the problem.
sadly, i think there's been too much change in TSRM etc between 5.2<->5.3,
so more functions would need to be backported afaict. maybe it'd be
better to try and figure out why the existing virtual_file_ex doesn't
like this filename, since it might affect other codepaths too?
rangda[/home/sean/Desktop/php-5.2.8] make :)
<snip>...
/bin/sh /home/sean/Desktop/php-5.2.8/libtool --silent --preserve-dup-deps --mode=compile gcc -Iext/zip/ -I/home/sean/Desktop/php-5.2.8/ext/zip/ -DPHP_ATOM_INC -I/home/sean/Desktop/php-5.2.8/include -I/home/sean/Desktop/php-5.2.8/main -I/home/sean/Desktop/php-5.2.8 -I/usr/include/libxml2 -I/home/sean/Desktop/php-5.2.8/ext/date/lib -I/home/sean/Desktop/php-5.2.8/TSRM -I/home/sean/Desktop/php-5.2.8/Zend -g -O0 -c /home/sean/Desktop/php-5.2.8/ext/zip/php_zip.c -o ext/zip/php_zip.lo
/home/sean/Desktop/php-5.2.8/ext/zip/php_zip.c:175:39: error: macro "tsrm_do_alloca" passed 2 arguments, but takes just 1
/home/sean/Desktop/php-5.2.8/ext/zip/php_zip.c: In function 'php_zip_realpath_r':
/home/sean/Desktop/php-5.2.8/ext/zip/php_zip.c:175: error: 'tsrm_do_alloca' undeclared (first use in this function)
/home/sean/Desktop/php-5.2.8/ext/zip/php_zip.c:175: error: (Each undeclared identifier is reported only once
/home/sean/Desktop/php-5.2.8/ext/zip/php_zip.c:175: error: for each function it appears in.)
/home/sean/Desktop/php-5.2.8/ext/zip/php_zip.c:207:35: error: macro "tsrm_free_alloca" passed 2 arguments, but takes just 1
/home/sean/Desktop/php-5.2.8/ext/zip/php_zip.c:207: error: 'tsrm_free_alloca' undeclared (first use in this function)
/home/sean/Desktop/php-5.2.8/ext/zip/php_zip.c:215:33: error: macro "tsrm_free_alloca" passed 2 arguments, but takes just 1
/home/sean/Desktop/php-5.2.8/ext/zip/php_zip.c: In function 'php_zip_extract_file':
/home/sean/Desktop/php-5.2.8/ext/zip/php_zip.c:487: warning: passing argument 6 of 'php_basename' from incompatible pointer type
make: *** [ext/zip/php_zip.lo] Error 1
rangda[/home/sean/Desktop/php-5.2.8] [2] :(
sean