Depacking with wrong password leaves 0 length files

Bug #70752

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 (profile)

Status:

Closed

Package:

Zip Related

PHP Version:

5.6.14

OS:

all

Private report:

CVE-ID:

None

[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

Pull Requests

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

-Status: Analyzed +Status: Closed

[2016-09-05 23:35 UTC] cmb@php.net

-Assigned To: +Assigned To: cmb