ID: 48763 Updated by: paj...@php.net Reported By: dani dot church at gmail dot com Status: Assigned Bug Type: Zip Related Operating System: CentOS 5 PHP Version: 5.2CVS-2009-07-01 (snap) Assigned To: pajoye New Comment:
Thanks for your patch! I have applied it to all branches and pecl. A pecl release will be done next week. Please note that the patch has been applied upstream as well (libzip repo). I will close the bug once the test is there too. Previous Comments: ------------------------------------------------------------------------ [2009-07-04 14:37:24] dani dot church at gmail dot com RalfBecker: In fact, one probable workaround, until this bug gets fixed, is to iterate through EVERY file in the ZipArchive, get the contents, and addFromString to put them back into the archive. By overwriting every single file in the archive (with its own contents), you won't trigger the bug. ------------------------------------------------------------------------ [2009-07-04 08:29:41] RalfBecker at outdoor-training dot de I can reproduce that bug with php5.2.9 under openSUSE11.0, thought I tried so far only oo3 *.odt files. It seems not to depend on the file, in fact I can not create a file, where I can replace content.xml with itself, without corrupting it. Ralf ------------------------------------------------------------------------ [2009-07-04 00:53:02] dani dot church at gmail dot com The patch, a PHP testbed, and a test ZIP file (empty.zip) can all be found at http://dchurch.ath.cx/phpzip/. The test ZIP is minimal and contains one empty file that uses a data descriptor. The PHP testbed takes this ZIP file, sets the modified flag by adding and removing a dummy file, and writes the results back to the browser. The ZIP file that PHP writes back to the browser is identical to the input file with the following exceptions: 1) The data descriptor, addresses 0x23-0x32 in the original file, is missing. The central directory starts at 0x33 in the original file, and at 0x23 in the modified file. 2) The central directory address, stored at 0x76 in the original file and 0x66 in the modified file, is updated from 0x33 to 0x23. 3) The local file header contains the flag 0x08 at address 0x06 to indicate that a data descriptor is present. This flag is cleared. 4) The central directory file header contains the flag 0x08 at address 0x3b (corresponding to 0x2b in the modified file), which is a copy of the same flag at 0x06. This flag SHOULD be cleared, but in the current CVS, it does not get cleared. The patch clears this flag. I don't have a test case for the other bug I found, since the if block at lines 173-185 seems to be something that isn't supposed to happen in the normal flow of execution. At the very least, I can't figure out a way to get to that point with ch_filename == NULL. However, if that block ever did get executed, it would result in a central directory entry with a listed filename length of 0 but the character "-" in the filename field-- again, an invalid ZIP file. ------------------------------------------------------------------------ [2009-07-03 17:35:24] paj...@php.net Ah thanks guys :) Do you have test cases for these two bugs (this one and the other you found while writing the patch)? It will help to valid the patch before I apply it. If you can post a link to the patch and the tests. Thanks for your work! ------------------------------------------------------------------------ [2009-07-03 02:33:42] dani dot church at gmail dot com You're absolutely right, that's the file with the bug. Both PHP 5.2.6 and 5.2.8+ produce malformed ZIP files, in slightly different ways, though only when UPDATING (not creating from scratch) ZIP files. The zip library does not write optional data descriptors, even if the input file has them. In 5.2.6, the "data descriptor exists" flag is set, even though there is no data descriptor. In 5.2.8+, the flag is (properly) cleared in the local file header, but not in the central directory. OpenOffice.org tolerates the bug in 5.2.6 but not the one in 5.2.8+. A patch to fix this bug (and another minor bug I found in the same area) follows: --- ext/zip/lib/zip_close.c.orig 2009-07-02 21:40:03.000000000 -0400 +++ ext/zip/lib/zip_close.c 2009-07-02 22:24:54.000000000 -0400 @@ -175,6 +175,7 @@ de.filename = strdup("-"); de.filename_len = 1; cd->entry[j].filename = "-"; + cd->entry[j].filename_len = 1; } else { de.filename = strdup(za->cdir->entry[i].filename); @@ -195,13 +196,14 @@ error = 1; break; } + memcpy(cd->entry+j, za->cdir->entry+i, sizeof(cd->entry[j])); if (de.bitflags & ZIP_GPBF_DATA_DESCRIPTOR) { de.crc = za->cdir->entry[i].crc; de.comp_size = za->cdir->entry[i].comp_size; de.uncomp_size = za->cdir->entry[i].uncomp_size; de.bitflags &= ~ZIP_GPBF_DATA_DESCRIPTOR; + cd->entry[j].bitflags &= ~ZIP_GPBF_DATA_DESCRIPTOR; } - memcpy(cd->entry+j, za->cdir->entry+i, sizeof(cd->entry[j])); } if (za->entry[i].ch_filename) { ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at http://bugs.php.net/48763 -- Edit this bug report at http://bugs.php.net/?id=48763&edit=1
