|
php.net | support | documentation | report a bug | advanced search | search howto | statistics | random bug | login |
[2020-03-14 19:19 UTC] bigshaq at wearehackerone dot com
Description: ------------ I was reading some of the zip files here: and stumbled across this line: https://github.com/php/php-src/blob/master/ext/zip/php_zip.c [snippet] ret = php_stream_mkdir(pathto, 0777, PHP_STREAM_MKDIR_RECURSIVE, NULL); [/snippet] which creates the directory (provided in the extractTo parameter) with 777 permissions. Moreover, during zip extraction(in php_zip_extract_file()): all of the directories are extracted with 777 permissions: [snippet] ret = php_stream_mkdir(file_dirname_fullpath, 0777, PHP_STREAM_MKDIR_RECURSIVE|REPORT_ERRORS, NULL); [/snippet] Anything that is not a directory is extracted with 664 permission (which allows everyone to read, i.e: rw-rw-r-- ) The combination of all of these things allows a malicious user on the same linux server to read the extracted files (because of the 664 permissions) and list the directories' info because of the 777 permissions, which allows easy navigation and make the attack surface even bigger. Recommendation: I would suggest one of the following: 1. Remove public access by default. Change the permissions to 660 and let the programmer use chmod() by himself if he really want to share those extracted files with everyone. 2. Alternatively, Set the default file permission to be a configureable thing (instead of hard-coded) Test script: --------------- <?php $zip = new ZipArchive; if ($zip->open('test.zip') === TRUE) { $zip->extractTo('/tmp/phpfuzz/outdir/'); $zip->close(); echo 'ok'; } else { echo 'failed'; } ?> Expected result: ---------------- (notice the permissions) drwxrwxr-x 3 ryanc ryanc 4096 Mar 14 11:26 outdir | | drwxrwxr-x 3 ryanc ryanc 4096 Mar 14 11:26 directory1 | -rw-rw---- 1 ryanc ryanc 4 Mar 14 11:26 secret1.txt | | drwxrwx--- 2 ryanc ryanc 4096 Mar 14 11:26 directory2 | -rw-rw---- 1 ryanc ryanc 4 Mar 14 11:26 secret2_recursive.txt Actual result: -------------- (notice the permissions) drwxrwxr-x 3 ryanc ryanc 4096 Mar 14 11:26 outdir | | drwxrwxr-x 3 ryanc ryanc 4096 Mar 14 11:26 directory1 | -rw-rw-r-- 1 ryanc ryanc 4 Mar 14 11:26 secret1.txt | | drwxrwxr-x 2 ryanc ryanc 4096 Mar 14 11:26 directory2 | -rw-rw-r-- 1 ryanc ryanc 4 Mar 14 11:26 secret2_recursive.txt PatchesPull RequestsHistoryAllCommentsChangesGit/SVN commits
|
|||||||||||||||||||||||||||
Copyright © 2001-2025 The PHP GroupAll rights reserved. |
Last updated: Fri Nov 07 06:00:01 2025 UTC |
sorry, this is the expected result: Expected result: ---------------- (notice the permissions) drwxrwx--- 3 ryanc ryanc 4096 Mar 14 11:26 outdir | | drwxrwx--- 3 ryanc ryanc 4096 Mar 14 11:26 directory1 | -rw-rw---- 1 ryanc ryanc 4 Mar 14 11:26 secret1.txt | | drwxrwx--- 2 ryanc ryanc 4096 Mar 14 11:26 directory2 | -rw-rw---- 1 ryanc ryanc 4 Mar 14 11:26 secret2_recursive.txtI don't really think is a security issue, perhaps it needs some documentation. extractTo honours umask, so this have to be set in the script. umask(0027); $z = new ZipArchive(); $z->open('ext/zip/tests/test.zip'); $z->extractTo('/tmp/foo'); ll /tmp/foo -rw-r-----. 1 remi remi 4 4 août 2005 bar -rw-r-----. 1 remi remi 8 6 juil. 2006 entry1.txt drwxr-x---. 2 remi remi 60 15 mars 07:30 foobar > 2. Alternatively, Set the default file permission to be a configureable thing (instead of hard-coded) So it is already under the user control.