Skip to content

Commit 7554a50

Browse files
danielebarbarodanielebarbaro
committed
feat(kmz): add archive limits and use temp_directory
extractAllFiles() read an archive with no ceiling on entry count or uncompressed size, so a KMZ declaring a handful of entries that expand into gigabytes filled the disk before anything noticed. Entry names went unchecked too. Both are now validated before a single byte is read, and either limit can be turned off by setting it to 0. The mkdir() return value was ignored, so a destination that could not be created produced a warning and then a confusing failure further down. It now throws with the path that could not be made. temp_directory has been in the config since the first release without anything reading it. extractAllFiles() takes an optional destination and falls back to that key, then to the system temp directory, giving each call a directory of its own. extractAllFiles() also stops throwing bare KmlException for a missing or unreadable archive and uses KmzExtractorException like the rest of the class. That is a narrowing, KmzExtractorException extends KmlException.
1 parent 15554e5 commit 7554a50

5 files changed

Lines changed: 354 additions & 36 deletions

File tree

‎README.md‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,13 @@ $extractor = new KmzExtractor();
261261
$files = $extractor->extractAllFiles('path/to/file.kmz', 'extraction/directory');
262262
```
263263

264-
`extractAllFiles()` returns the list of entry names it wrote. It extracts whatever the archive contains, so point it at a directory you control and treat uploaded archives as untrusted input.
264+
`extractAllFiles()` returns the list of entry names it wrote. Leave the destination out and it writes to a directory of its own under `temp_directory`, or under the system temp directory when that is null:
265+
266+
```php
267+
$files = $extractor->extractAllFiles('path/to/file.kmz');
268+
```
269+
270+
Archives are checked before anything is read out of them. An archive is rejected when it declares more than `max_archive_entries` entries, when its entries add up to more than `max_uncompressed_size` bytes uncompressed, or when any entry name is absolute or contains `..` and would therefore write outside the destination. Set either limit to `0` to turn it off.
265271

266272
## Error handling
267273

@@ -312,8 +318,14 @@ return [
312318
'http://earth.google.com/kml/2.0',
313319
],
314320

315-
// Reserved for KMZ extraction. Not used yet.
321+
// Where extractAllFiles() writes when given no destination.
322+
// null means the system temp directory.
316323
'temp_directory' => null,
324+
325+
// Ceilings applied to a KMZ before anything is read out of it.
326+
// Set either to 0 to disable it.
327+
'max_archive_entries' => 5000,
328+
'max_uncompressed_size' => 256 * 1024 * 1024,
317329
];
318330
```
319331

‎config/kml-parser.php‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,25 @@
3636
| Temporary Directory
3737
|--------------------------------------------------------------------------
3838
|
39-
| This value determines the temporary directory used for extracting KMZ files.
40-
| If null, the system temp directory will be used.
39+
| Where KmzExtractor::extractAllFiles() writes when the caller names no
40+
| destination. Each call gets its own directory underneath it. If null,
41+
| the system temp directory is used.
4142
|
4243
*/
4344
'temp_directory' => null,
45+
46+
/*
47+
|--------------------------------------------------------------------------
48+
| Archive Limits
49+
|--------------------------------------------------------------------------
50+
|
51+
| A KMZ is a ZIP, and a ZIP can declare a handful of entries that expand
52+
| into far more than the machine has. An archive breaching either ceiling
53+
| is rejected before anything is read out of it. A real KMZ is a KML plus
54+
| its icons, nowhere near either number. Set one to 0 to disable it.
55+
|
56+
*/
57+
'max_archive_entries' => 5000,
58+
59+
'max_uncompressed_size' => 256 * 1024 * 1024,
4460
];

‎src/Exceptions/KmzExtractorException.php‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,24 @@ public static function invalidZipFile(string $path): self
2323
{
2424
return new self("Invalid KMZ file: {$path}");
2525
}
26+
27+
public static function tooManyEntries(int $count, int $max): self
28+
{
29+
return new self("KMZ archive holds {$count} entries, more than the {$max} allowed");
30+
}
31+
32+
public static function archiveTooLarge(int $max): self
33+
{
34+
return new self("KMZ archive expands to more than the {$max} bytes allowed");
35+
}
36+
37+
public static function unsafeEntry(string $name): self
38+
{
39+
return new self("KMZ archive holds an entry that would escape the destination: {$name}");
40+
}
41+
42+
public static function destinationNotWritable(string $path): self
43+
{
44+
return new self("Unable to create the extraction directory: {$path}");
45+
}
2646
}

‎src/KmzExtractor.php‎

Lines changed: 155 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -2,40 +2,45 @@
22

33
namespace PlinCode\KmlParser;
44

5-
use PlinCode\KmlParser\Exceptions\KmlException;
65
use PlinCode\KmlParser\Exceptions\KmzExtractorException;
6+
use PlinCode\KmlParser\Traits\ReadsPackageConfig;
77
use ZipArchive;
88

99
class KmzExtractor
1010
{
11+
use ReadsPackageConfig;
12+
13+
/**
14+
* Ceilings applied to an archive before anything is read out of it.
15+
*
16+
* A KMZ is a ZIP, and a ZIP can declare a handful of entries that expand
17+
* into far more than the machine has. These are deliberately generous: a
18+
* real KMZ is a KML plus its icons, nowhere near either limit.
19+
*/
20+
public const DEFAULT_MAX_ENTRIES = 5000;
21+
22+
public const DEFAULT_MAX_UNCOMPRESSED_SIZE = 268435456; // 256 MB
23+
1124
/**
1225
* Extract KML content from a KMZ file
26+
*
27+
* @throws KmzExtractorException
1328
*/
1429
public function extractKmlContent(string $path): string
1530
{
16-
if (! file_exists($path)) {
17-
throw KmzExtractorException::fileNotFound($path);
18-
}
19-
20-
$zip = new ZipArchive;
21-
if ($zip->open($path) !== true) {
22-
throw KmzExtractorException::invalidZipFile($path);
23-
}
31+
$zip = $this->open($path);
2432

2533
try {
26-
$kmlFiles = array_filter(
27-
array_map(
28-
fn (int $i) => $zip->getNameIndex($i),
29-
range(0, $zip->numFiles - 1)
30-
),
31-
fn (string $filename) => pathinfo($filename, PATHINFO_EXTENSION) === 'kml'
32-
);
33-
34-
if (empty($kmlFiles)) {
34+
$this->guardArchive($zip);
35+
36+
$kmlIndex = $this->firstKmlIndex($zip);
37+
38+
if ($kmlIndex === null) {
3539
throw KmzExtractorException::noKmlFound();
3640
}
3741

38-
$kmlContent = $zip->getFromIndex(array_key_first($kmlFiles));
42+
$kmlContent = $zip->getFromIndex($kmlIndex);
43+
3944
if ($kmlContent === false) {
4045
throw KmzExtractorException::failedToExtract('Failed to read KML file from archive');
4146
}
@@ -49,32 +54,150 @@ public function extractKmlContent(string $path): string
4954
/**
5055
* Extract all files from KMZ archive
5156
*
52-
* @throws KmlException If the KMZ file cannot be found or opened
57+
* Without a destination the files go to the configured temp_directory, or
58+
* to the system temp directory, in a directory of their own.
59+
*
60+
* @return array<int, string> the entry names that were written
61+
*
62+
* @throws KmzExtractorException
5363
*/
54-
public function extractAllFiles(string $kmzPath, string $destination): array
64+
public function extractAllFiles(string $kmzPath, ?string $destination = null): array
5565
{
56-
if (! file_exists($kmzPath)) {
57-
throw new KmlException("KMZ file not found: {$kmzPath}");
66+
$destination ??= $this->defaultDestination();
67+
68+
$zip = $this->open($kmzPath);
69+
70+
try {
71+
$this->guardArchive($zip);
72+
73+
/*
74+
* The warning mkdir() raises carries less than the exception
75+
* thrown below, and an application turning warnings into
76+
* exceptions would otherwise get that one instead of ours. The
77+
* return value is what is acted on, the second is_dir() covers
78+
* another process winning the race.
79+
*/
80+
if (! is_dir($destination) && ! @mkdir($destination, 0755, true) && ! is_dir($destination)) {
81+
throw KmzExtractorException::destinationNotWritable($destination);
82+
}
83+
84+
if (! $zip->extractTo($destination)) {
85+
throw KmzExtractorException::failedToExtract("Unable to extract the archive into {$destination}");
86+
}
87+
88+
return $this->entryNames($zip);
89+
} finally {
90+
$zip->close();
91+
}
92+
}
93+
94+
/**
95+
* The directory extractAllFiles() writes to when the caller names none.
96+
*/
97+
public function defaultDestination(): string
98+
{
99+
$configured = $this->packageConfig('kml-parser.temp_directory', null);
100+
$base = is_string($configured) && $configured !== '' ? $configured : sys_get_temp_dir();
101+
102+
return rtrim($base, '/\\').DIRECTORY_SEPARATOR.'kml-parser-'.uniqid();
103+
}
104+
105+
/**
106+
* @throws KmzExtractorException
107+
*/
108+
protected function open(string $path): ZipArchive
109+
{
110+
if (! file_exists($path)) {
111+
throw KmzExtractorException::fileNotFound($path);
58112
}
59113

60114
$zip = new ZipArchive;
61-
if ($zip->open($kmzPath) !== true) {
62-
throw new KmlException("Unable to open KMZ file: {$kmzPath}");
115+
116+
if ($zip->open($path) !== true) {
117+
throw KmzExtractorException::invalidZipFile($path);
63118
}
64119

65-
if (! file_exists($destination)) {
66-
mkdir($destination, 0755, true);
120+
return $zip;
121+
}
122+
123+
/**
124+
* Reject an archive that is too large to trust before reading anything out
125+
* of it, and reject any entry whose name would escape the destination.
126+
*
127+
* @throws KmzExtractorException
128+
*/
129+
protected function guardArchive(ZipArchive $zip): void
130+
{
131+
$maxEntries = (int) $this->packageConfig('kml-parser.max_archive_entries', self::DEFAULT_MAX_ENTRIES);
132+
$maxSize = (int) $this->packageConfig('kml-parser.max_uncompressed_size', self::DEFAULT_MAX_UNCOMPRESSED_SIZE);
133+
134+
if ($maxEntries > 0 && $zip->numFiles > $maxEntries) {
135+
throw KmzExtractorException::tooManyEntries($zip->numFiles, $maxEntries);
67136
}
68137

69-
$zip->extractTo($destination);
138+
$total = 0;
70139

71-
$extractedFiles = [];
72140
for ($i = 0; $i < $zip->numFiles; $i++) {
73-
$extractedFiles[] = $zip->getNameIndex($i);
141+
$stat = $zip->statIndex($i);
142+
143+
if ($stat === false) {
144+
throw KmzExtractorException::failedToExtract("Unable to read entry {$i} of the archive");
145+
}
146+
147+
$this->guardEntryName((string) $stat['name']);
148+
149+
$total += (int) $stat['size'];
150+
151+
if ($maxSize > 0 && $total > $maxSize) {
152+
throw KmzExtractorException::archiveTooLarge($maxSize);
153+
}
74154
}
155+
}
75156

76-
$zip->close();
157+
/**
158+
* @throws KmzExtractorException
159+
*/
160+
protected function guardEntryName(string $name): void
161+
{
162+
if (str_starts_with($name, '/') || str_starts_with($name, '\\') || preg_match('#^[A-Za-z]:[\\\\/]#', $name) === 1) {
163+
throw KmzExtractorException::unsafeEntry($name);
164+
}
165+
166+
foreach (preg_split('#[\\\\/]#', $name) ?: [] as $segment) {
167+
if ($segment === '..') {
168+
throw KmzExtractorException::unsafeEntry($name);
169+
}
170+
}
171+
}
172+
173+
protected function firstKmlIndex(ZipArchive $zip): ?int
174+
{
175+
for ($i = 0; $i < $zip->numFiles; $i++) {
176+
$name = $zip->getNameIndex($i);
177+
178+
if ($name !== false && strtolower(pathinfo($name, PATHINFO_EXTENSION)) === 'kml') {
179+
return $i;
180+
}
181+
}
182+
183+
return null;
184+
}
185+
186+
/**
187+
* @return array<int, string>
188+
*/
189+
protected function entryNames(ZipArchive $zip): array
190+
{
191+
$names = [];
192+
193+
for ($i = 0; $i < $zip->numFiles; $i++) {
194+
$name = $zip->getNameIndex($i);
195+
196+
if ($name !== false) {
197+
$names[] = $name;
198+
}
199+
}
77200

78-
return $extractedFiles;
201+
return $names;
79202
}
80203
}

0 commit comments

Comments
 (0)