Skip to content

Commit 151c93d

Browse files
committed
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.
1 parent e45d0d9 commit 151c93d

2 files changed

Lines changed: 51 additions & 0 deletions

File tree

src/Zip.php

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ public function contents()
8989
*
9090
* @see contents()
9191
* @throws ArchiveIOException
92+
* @throws ArchiveCorruptedException
9293
* @return FileInfo[]
9394
*/
9495
public function yieldContents()
@@ -129,6 +130,7 @@ public function yieldContents()
129130
* @param string $exclude a regular expression of files to exclude
130131
* @param string $include a regular expression of files to include
131132
* @throws ArchiveIOException
133+
* @throws ArchiveCorruptedException
132134
* @return FileInfo[]
133135
*/
134136
public function extract($outdir, $strip = '', $exclude = '', $include = '')
@@ -538,6 +540,7 @@ public function save($file)
538540
* This key-value list contains general information about the ZIP file
539541
*
540542
* @return array
543+
* @throws ArchiveCorruptedException when the file is not a valid ZIP archive
541544
*/
542545
protected function readCentralDir()
543546
{
@@ -551,20 +554,33 @@ protected function readCentralDir()
551554
@fseek($this->fh, $size - $maximum_size);
552555
$pos = ftell($this->fh);
553556
$bytes = 0x00000000;
557+
$found = false;
554558

555559
while ($pos < $size) {
556560
$byte = @fread($this->fh, 1);
557561
$bytes = (($bytes << 8) & 0xFFFFFFFF) | ord($byte);
558562
if ($bytes == 0x504b0506) {
563+
$found = true;
559564
break;
560565
}
561566
$pos++;
562567
}
563568

569+
if (!$found) {
570+
throw new ArchiveCorruptedException(
571+
'End of central directory signature not found - not a valid ZIP file'
572+
);
573+
}
574+
564575
$data = unpack(
565576
'vdisk/vdisk_start/vdisk_entries/ventries/Vsize/Voffset/vcomment_size',
566577
fread($this->fh, 18)
567578
);
579+
if ($data === false) {
580+
throw new ArchiveCorruptedException(
581+
'Could not read end of central directory record'
582+
);
583+
}
568584

569585
if ($data['comment_size'] != 0) {
570586
$centd['comment'] = fread($this->fh, $data['comment_size']);

tests/ZipTestCase.php

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,41 @@ public function testMissing()
5252
$tar->open('nope.zip');
5353
}
5454

55+
/**
56+
* Feeding a file without an End-of-Central-Directory signature must raise
57+
* ArchiveCorruptedException, not bubble up PHP warnings from unpack().
58+
*/
59+
public function testNotAZip()
60+
{
61+
$tmp = tempnam(sys_get_temp_dir(), 'phpa-bad-');
62+
file_put_contents($tmp, str_repeat('this is not a zip file ', 50));
63+
try {
64+
$zip = new Zip();
65+
$zip->open($tmp);
66+
$this->expectException(ArchiveCorruptedException::class);
67+
iterator_to_array($zip->yieldContents());
68+
} finally {
69+
@unlink($tmp);
70+
}
71+
}
72+
73+
/**
74+
* A truncated file (smaller than the EOCD record) must also fail cleanly.
75+
*/
76+
public function testTruncatedFile()
77+
{
78+
$tmp = tempnam(sys_get_temp_dir(), 'phpa-trunc-');
79+
file_put_contents($tmp, "PK\x03\x04");
80+
try {
81+
$zip = new Zip();
82+
$zip->open($tmp);
83+
$this->expectException(ArchiveCorruptedException::class);
84+
iterator_to_array($zip->yieldContents());
85+
} finally {
86+
@unlink($tmp);
87+
}
88+
}
89+
5590
/**
5691
* simple test that checks that the given filenames and contents can be grepped from
5792
* the uncompressed zip stream

0 commit comments

Comments
 (0)