diff --git a/src/Zip.php b/src/Zip.php index 7b8030c..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 = ''; @@ -89,6 +93,7 @@ public function contents() * * @see contents() * @throws ArchiveIOException + * @throws ArchiveCorruptedException * @return FileInfo[] */ public function yieldContents() @@ -129,6 +134,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 = '') @@ -478,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', @@ -538,6 +544,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() { @@ -550,21 +557,31 @@ 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 !== self::SIG_END_OF_CENTRAL_DIR) { + 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']); @@ -912,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 @@ -959,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 diff --git a/tests/ZipTestCase.php b/tests/ZipTestCase.php index 7d4f7a5..eb27e0b 100644 --- a/tests/ZipTestCase.php +++ b/tests/ZipTestCase.php @@ -52,6 +52,43 @@ 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 = 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 { + $this->expectException(ArchiveCorruptedException::class); + iterator_to_array($zip->yieldContents()); + } finally { + try { $zip->close(); } catch (\Exception $e) { /* already errored */ } + @unlink($tmp); + } + } + + /** + * A truncated file (smaller than the EOCD record) must also fail cleanly. + */ + public function testTruncatedFile() + { + $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 { + $this->expectException(ArchiveCorruptedException::class); + iterator_to_array($zip->yieldContents()); + } finally { + try { $zip->close(); } catch (\Exception $e) { /* already errored */ } + @unlink($tmp); + } + } + /** * simple test that checks that the given filenames and contents can be grepped from * the uncompressed zip stream