PHP 5.2.7 and ZipArchive::extractTo()
December 5th, 2008 | by Stefan Esser |
165 days ago I was sitting at a customer’s place and were auditing a large scale web application. The audit was mainly a blackbox penetration test to check if an attacker could attack the application with zero knowledge. However when we found something interesting we were also able to look into the code and check for the exact problem.
The web application in question was one of those big sites that mainly work on user generated content. One of the features of this site is that users can upload ZIP archive files that are then unpacked and added to the users “workbench”. So without knowing anything about the internal implementation I started throwing malicious and broken ZIP files at the application. I created them with my favourite hex editor, by just changing random (or not so random) bytes.
Of course I also changed the filenames inside the archive, added some ../ components and tried if the stone old bug of directory traversal during unpacking worked. And guess what… It worked… I suddenly was able to write anywhere the web-server user was allowed to write to, which is very nice when you can drop new .PHP files to some writable directory inside the document root.
Then the next step was to look at the actual implementation and find out what cause the bug, to give directions to the developers how to fix the problem. It turned out that the application was just calling the ZipArchive::extractTo() function to unpack the files to some temporary directory. This directory was then traversed and every single file or subdirectory added to the “workbench”, followed by a deletion of the temporary directory.
Basically that meant the directory traversal vulnerability was inside ZipArchive::extractTo(). Therefore I reported the problem to security@php.net on the same afternoon I found it, which was 165 days, nearly half a year ago. I also released an advisory about this yesterday, which is available here. The release actually happend BEFORE the official announcement of PHP 5.2.7 on the PHP website, but because I provided the download link to the tarball within the advisory everyone reading it was already able to download and update. Well actually the frontpage of PHP.net still lacks the announcement.
Unfortunately when PHP 5.2.7 was released, it came with a release announcement and a changelog that do not document the security related change in the ZipArchive::extractTo() method at all, which is very unfortunate because from looking at the other reported security problems, the ZipArchive problem seems the most problematic one in real world situations.
For the future, the PHP developers should create some protected wiki page where they write down all the reported security bugs. This might help them not to forget about security problems when they release half a year later…
Ohh, btw… With the release of PHP 5.2.7 I also ported Suhosin-Patch to it. Additionally the new Suhosin release contains a simple bugfix for unaligned memory access inside the Zend Memory Manager protection. A bug that stopped Suhosin from working at all when you used a compiler like GCC in combination with an architecture like Sparc.
12 Responses to “PHP 5.2.7 and ZipArchive::extractTo()”
By wesley on Dec 5, 2008 | Reply
So, you’re not really syaing it, but it has been fixes in 5.2.7 right?
By Stefan Esser on Dec 5, 2008 | Reply
The ext/zip maintainer commited some changes that will flatten the names in the ZIP archive before unpacking it.
This should fix the problem.
By Pierre on Dec 5, 2008 | Reply
The maintainer committed a patch to fix this issue. The patch has been sent to Stefan without answer/reply.
However Stefan was nice enough to send him a reminder during the ZendConf while saying he had done nothing to fix the issue in XXX days. So the maintainer took him in a corner during ICP in Mainz
Short version: it is fixed yes, a pecl release will follow today
Cheers,
By Basil Hussain on Dec 5, 2008 | Reply
Your post got me thinking about whether a third-party Zip library included in some open-source apps my company uses is vulnerable to directory traversal. I found that it actually is!
The library is PclZip (http://www.phpconcept.net/pclzip/index.en.php), which I believe is quite popular with many open-source PHP projects (as a quick search on koders.com will show).
The amusing thing is, it has a feature in recent versions which purports to mitigate directory traversal when extracting files, but it is actually completely useless against relative paths (as opposed to absolute). Basically, it does not expand paths before comparing to the restriction path. So, if you tell it to extract to ‘/var/www/myapp/temp/unzip’ and also restrict to that dir, all it ends up doing is comparing that to ‘/var/www/myapp/temp/unzip/../foo.txt’, which of course passes the check!
I will try to notify the author.
By Hirvine on Dec 5, 2008 | Reply
Good lord, I’ve never thought of hacking the path with an hex editor o.0. Why? It never occured to me. Good thinking, but poor it actually works. The ‘directory-var’ is such a very old nasty hack I thought ‘PHP Developpers’ should know.
Good thing ‘guardians’ like yourselves finds these issues. … I oughta try it out myself.
By bjori on Dec 6, 2008 | Reply
The release announcement was available on php.net long before the release announcement was sent to php-announcement@.
However due to a typo in the news entry it did not show up on the frontpage, and the news entry was only available via the archive at php.net/archive.
This was fixed in CVS _before_ the php-announcement@ email was sent out but may have taken up to 1hour to synchronize to all our 120 mirrors which, unfortunately, we have no control over.
By bjori on Dec 6, 2008 | Reply
The news entry has been updated with a note about the fix.
In the future, please report these kind of issues to php-webmaster@lists.php.net. It is ridiculously annoying needing to hear about this kind of issues from 3rd parties.. pointing me to “random” *blog*.
By Stefan Esser on Dec 6, 2008 | Reply
@Bjori:
I am not blaming PHP devs for announcing PHP 5.2.7 too late on the front page. I might blame them for putting advertising links like that unrelated PHP Advent crap on the frontpage.
Whatever… Just mailing corrections to php-webmaster@lists.php.net does not fix the problem. The problem is that obviously security holes got forgotten, otherwise they would have been announced in the changelog. This should not happen. One way to fix that would be to have a protected wiki page where all security holes are written down the moment they are reported.
Knowing the PHP developers, just mailing the webmasters of php.net will not result in such changes. The only way to achieve these changes is to discuss them publicly.
By phpBuddy on Dec 6, 2008 | Reply
@ Hirvine
That’s the Problem with security holes like this - the attackers know about these possibilities.
I myself use ZipArchive in several projects, so i guess it’s time for some updates.
Happy St. Nicholas’ day!