From 151c93dd4b30b6ea9792d51f35929b48e72421c5 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 12 May 2026 23:02:22 +0000 Subject: [PATCH 1/4] validate ZIP central directory and throw on corruption The readCentralDir() scan for the End-of-Central-Directory signature silently fell through when it was missing, leaving the file pointer past EOF and feeding null to unpack(). That surfaced as PHP warnings ("unpack(): Type V: not enough input") rather than a meaningful error. Detect both the missing EOCD signature and a failed unpack() and raise ArchiveCorruptedException instead, matching the error handling used elsewhere in the package. Added tests covering a non-ZIP payload and a file too short to hold an EOCD record. --- src/Zip.php | 16 ++++++++++++++++ tests/ZipTestCase.php | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/src/Zip.php b/src/Zip.php index 7b8030c..246b27a 100644 --- a/src/Zip.php +++ b/src/Zip.php @@ -89,6 +89,7 @@ public function contents() * * @see contents() * @throws ArchiveIOException + * @throws ArchiveCorruptedException * @return FileInfo[] */ public function yieldContents() @@ -129,6 +130,7 @@ public function yieldContents() * @param string $exclude a regular expression of files to exclude * @param string $include a regular expression of files to include * @throws ArchiveIOException + * @throws ArchiveCorruptedException * @return FileInfo[] */ public function extract($outdir, $strip = '', $exclude = '', $include = '') @@ -538,6 +540,7 @@ public function save($file) * This key-value list contains general information about the ZIP file * * @return array + * @throws ArchiveCorruptedException when the file is not a valid ZIP archive */ protected function readCentralDir() { @@ -551,20 +554,33 @@ protected function readCentralDir() @fseek($this->fh, $size - $maximum_size); $pos = ftell($this->fh); $bytes = 0x00000000; + $found = false; while ($pos < $size) { $byte = @fread($this->fh, 1); $bytes = (($bytes << 8) & 0xFFFFFFFF) | ord($byte); if ($bytes == 0x504b0506) { + $found = true; break; } $pos++; } + if (!$found) { + throw new ArchiveCorruptedException( + 'End of central directory signature not found - not a valid ZIP file' + ); + } + $data = unpack( 'vdisk/vdisk_start/vdisk_entries/ventries/Vsize/Voffset/vcomment_size', fread($this->fh, 18) ); + if ($data === false) { + throw new ArchiveCorruptedException( + 'Could not read end of central directory record' + ); + } if ($data['comment_size'] != 0) { $centd['comment'] = fread($this->fh, $data['comment_size']); diff --git a/tests/ZipTestCase.php b/tests/ZipTestCase.php index 7d4f7a5..8511b1f 100644 --- a/tests/ZipTestCase.php +++ b/tests/ZipTestCase.php @@ -52,6 +52,41 @@ public function testMissing() $tar->open('nope.zip'); } + /** + * Feeding a file without an End-of-Central-Directory signature must raise + * ArchiveCorruptedException, not bubble up PHP warnings from unpack(). + */ + public function testNotAZip() + { + $tmp = tempnam(sys_get_temp_dir(), 'phpa-bad-'); + file_put_contents($tmp, str_repeat('this is not a zip file ', 50)); + try { + $zip = new Zip(); + $zip->open($tmp); + $this->expectException(ArchiveCorruptedException::class); + iterator_to_array($zip->yieldContents()); + } finally { + @unlink($tmp); + } + } + + /** + * A truncated file (smaller than the EOCD record) must also fail cleanly. + */ + public function testTruncatedFile() + { + $tmp = tempnam(sys_get_temp_dir(), 'phpa-trunc-'); + file_put_contents($tmp, "PK\x03\x04"); + try { + $zip = new Zip(); + $zip->open($tmp); + $this->expectException(ArchiveCorruptedException::class); + iterator_to_array($zip->yieldContents()); + } finally { + @unlink($tmp); + } + } + /** * simple test that checks that the given filenames and contents can be grepped from * the uncompressed zip stream From ed2fd8b5ff4fa9d0cabbee5436b1fd7df940435d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 12 May 2026 23:06:29 +0000 Subject: [PATCH 2/4] simplify EOCD-not-found check Drop the $found flag: after the loop, $bytes can only equal the EOCD signature if we broke out, since the check runs on every iteration. --- src/Zip.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Zip.php b/src/Zip.php index 246b27a..aaf0756 100644 --- a/src/Zip.php +++ b/src/Zip.php @@ -554,19 +554,17 @@ protected function readCentralDir() @fseek($this->fh, $size - $maximum_size); $pos = ftell($this->fh); $bytes = 0x00000000; - $found = false; while ($pos < $size) { $byte = @fread($this->fh, 1); $bytes = (($bytes << 8) & 0xFFFFFFFF) | ord($byte); if ($bytes == 0x504b0506) { - $found = true; break; } $pos++; } - if (!$found) { + if ($bytes != 0x504b0506) { throw new ArchiveCorruptedException( 'End of central directory signature not found - not a valid ZIP file' ); From 07d07d2f7a41eb7d0c16831843a52f507dcebc24 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 12 May 2026 23:09:52 +0000 Subject: [PATCH 3/4] extract PK signatures into class constants Three ZIP record signatures were sprinkled through the file as raw byte literals (one as a uint32 in the EOCD scan, three more as "\x50\x4b..." strings on write). Pull them out as SIG_* class constants and convert the EOCD scan to a 4-byte sliding string buffer so reads and writes share a single source of truth. --- src/Zip.php | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/Zip.php b/src/Zip.php index aaf0756..e280bef 100644 --- a/src/Zip.php +++ b/src/Zip.php @@ -17,6 +17,10 @@ class Zip extends Archive { const LOCAL_FILE_HEADER_CRC_OFFSET = 14; + const SIG_LOCAL_FILE_HEADER = "\x50\x4b\x03\x04"; + const SIG_CENTRAL_FILE_HEADER = "\x50\x4b\x01\x02"; + const SIG_END_OF_CENTRAL_DIR = "\x50\x4b\x05\x06"; + protected $file = ''; protected $fh; protected $memory = ''; @@ -480,7 +484,7 @@ public function close() $this->writebytes($ctrldir); // write end of central directory record - $this->writebytes("\x50\x4b\x05\x06"); // end of central dir signature + $this->writebytes(self::SIG_END_OF_CENTRAL_DIR); $this->writebytes(pack('v', 0)); // number of this disk $this->writebytes(pack('v', 0)); // number of the disk with the start of the central directory $this->writebytes(pack('v', @@ -553,18 +557,17 @@ protected function readCentralDir() @fseek($this->fh, $size - $maximum_size); $pos = ftell($this->fh); - $bytes = 0x00000000; + $bytes = ''; while ($pos < $size) { - $byte = @fread($this->fh, 1); - $bytes = (($bytes << 8) & 0xFFFFFFFF) | ord($byte); - if ($bytes == 0x504b0506) { + $bytes = substr($bytes . (string)@fread($this->fh, 1), -4); + if ($bytes === self::SIG_END_OF_CENTRAL_DIR) { break; } $pos++; } - if ($bytes != 0x504b0506) { + if ($bytes !== self::SIG_END_OF_CENTRAL_DIR) { throw new ArchiveCorruptedException( 'End of central directory signature not found - not a valid ZIP file' ); @@ -926,7 +929,7 @@ protected function makeCentralFileRecord($offset, $ts, $crc, $len, $clen, $name, list($name, $extra) = $this->encodeFilename($name); - $header = "\x50\x4b\x01\x02"; // central file header signature + $header = self::SIG_CENTRAL_FILE_HEADER; $header .= pack('v', 14); // version made by - VFAT $header .= pack('v', 20); // version needed to extract - 2.0 $header .= pack('v', 0); // general purpose flag - no flags set @@ -973,7 +976,7 @@ protected function makeLocalFileHeader($ts, $crc, $len, $clen, $name, $comp = nu list($name, $extra) = $this->encodeFilename($name); - $header = "\x50\x4b\x03\x04"; // local file header signature + $header = self::SIG_LOCAL_FILE_HEADER; $header .= pack('v', 20); // version needed to extract - 2.0 $header .= pack('v', 0); // general purpose flag - no flags set $header .= pack('v', $comp); // compression method - deflate|none From 710e0df3a48b6ca5b852c5cffde8e76382c61944 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 12 May 2026 23:21:16 +0000 Subject: [PATCH 4/4] make corruption tests Windows-safe CI failed only on PHP 8.0 / windows-latest. Replace tempnam() with the sys_get_temp_dir() + md5(uniqid()) pattern the rest of the suite uses, and explicitly close the Zip handle in finally so Windows can unlink the temp file (open handles block deletion). --- tests/ZipTestCase.php | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/ZipTestCase.php b/tests/ZipTestCase.php index 8511b1f..eb27e0b 100644 --- a/tests/ZipTestCase.php +++ b/tests/ZipTestCase.php @@ -58,14 +58,15 @@ public function testMissing() */ public function testNotAZip() { - $tmp = tempnam(sys_get_temp_dir(), 'phpa-bad-'); + $tmp = sys_get_temp_dir() . '/phpa-bad-' . md5(uniqid('', true)) . '.bin'; file_put_contents($tmp, str_repeat('this is not a zip file ', 50)); + $zip = new Zip(); + $zip->open($tmp); try { - $zip = new Zip(); - $zip->open($tmp); $this->expectException(ArchiveCorruptedException::class); iterator_to_array($zip->yieldContents()); } finally { + try { $zip->close(); } catch (\Exception $e) { /* already errored */ } @unlink($tmp); } } @@ -75,14 +76,15 @@ public function testNotAZip() */ public function testTruncatedFile() { - $tmp = tempnam(sys_get_temp_dir(), 'phpa-trunc-'); + $tmp = sys_get_temp_dir() . '/phpa-trunc-' . md5(uniqid('', true)) . '.bin'; file_put_contents($tmp, "PK\x03\x04"); + $zip = new Zip(); + $zip->open($tmp); try { - $zip = new Zip(); - $zip->open($tmp); $this->expectException(ArchiveCorruptedException::class); iterator_to_array($zip->yieldContents()); } finally { + try { $zip->close(); } catch (\Exception $e) { /* already errored */ } @unlink($tmp); } }