php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #70752 Depacking with wrong password leaves 0 length files
Submitted: 2015-10-20 20:26 UTC Modified: 2016-09-05 23:35 UTC
Votes:1
Avg. Score:5.0 ± 0.0
Reproduced:1 of 1 (100.0%)
Same Version:1 (100.0%)
Same OS:1 (100.0%)
From: furun at arcor dot de Assigned: cmb
Status: Closed Package: Zip Related
PHP Version: 5.6.14 OS: all
Private report: No CVE-ID:
 [2015-10-20 20:26 UTC] furun at arcor dot de
Description:
------------
---
From manual page: http://www.php.net/ziparchive.setpassword
---

If a file is de-packed with ZipArchive::setPassword, and the password is wrong, it leafs a file with 0 length. In case a file exist already with the same name like the unpacked one, this file will be overwritten with 0 length content.

This behavior is unclean. In case a password is wrong, no files should be created or changed.

For Example:
If ZipArchive is used to update content in a secure way, a wrong password would destroy the existing files. Exactly what the password protection of the updater file should avoid.



And, There is no way to check a password before using it (... i guess)




Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2015-11-17 02:41 UTC] nhojohl at gmail dot com
Do you have a test script that replicates the behavior? I just test it and it worked without any issues.

Thanks!
 [2015-11-17 20:59 UTC] furun at arcor dot de
Thanks for the reply.

Sorry for the missing test code.

For testing:
1. Create a ZIP file like "C:\Temp\Test.zip", 
   with a file inside like "Test.txt", 
   with a password encrypted like "pass".
2. Run the test-code Zip_Test().
3. The password is wrong, and you should have a file named "Test.txt" with the length 0 in "C:\Temp\".
4. It create the 'ERROR: extractTo' Error.

5. If "C:\Temp\Test.txt" already exists and has content, it will be overwritten.

tested on xampp, PHP Version 5.6.3, on windows 7



<?php

Zip_Test();

function Zip_Test () {

	$filePath = 'C:\\Temp\\Test.zip';
	$destinationDirectory = 'C:\\Temp\\';
	$password = 'pass_wrong';

	$zip = new ZipArchive;

	if ($zip->open($filePath) !== true)                  { print ('ERROR: open'); return; }
	if ($zip->setPassword($password) !== true)           { print ('ERROR: setPassword'); return; }
	if ($zip->extractTo($destinationDirectory) !== true) { print ('ERROR: extractTo'); return; }

	$zip->close();
	
}
 [2015-11-17 21:30 UTC] furun at arcor dot de
ps:

...The argument is, if the password is wrong, it should not create/change any files.
 [2016-09-05 16:10 UTC] cmb@php.net
-Status: Open +Status: Analyzed
 [2016-09-05 16:10 UTC] cmb@php.net
I can confirm this issue. The problem is that libzip will only
check the password when the entry is actually opened. However, PHP
will first open the output stream, and only then call zip_fopen()[1],
so an empty file remains.

Simply changing the order (i.e. first zip_open() then
php_stream_open_wrapper()) should solve the issue.

[1] <https://github.com/php/php-src/blob/PHP-7.0.10/ext/zip/php_zip.c#L241-L253>
 [2016-09-05 23:34 UTC] cmb@php.net
Automatic comment on behalf of cmbecker69@gmx.de
Revision: http://git.php.net/?p=php-src.git;a=commit;h=8aad3131a1d00e191db1b3b27aed6e7fae269f13
Log: Fix #70752: Depacking with wrong password leaves 0 length files
 [2016-09-05 23:34 UTC] cmb@php.net
-Status: Analyzed +Status: Closed
 [2016-09-05 23:35 UTC] cmb@php.net
-Assigned To: +Assigned To: cmb
 [2016-10-17 10:08 UTC] bwoebi@php.net
Automatic comment on behalf of cmbecker69@gmx.de
Revision: http://git.php.net/?p=php-src.git;a=commit;h=8aad3131a1d00e191db1b3b27aed6e7fae269f13
Log: Fix #70752: Depacking with wrong password leaves 0 length files
 
PHP Copyright © 2001-2017 The PHP Group
All rights reserved.
Last updated: Fri Feb 24 01:01:37 2017 UTC