feat(kmz): add archive limits and use temp_directory

README.md

@@ -261,7 +261,13 @@ $extractor = new KmzExtractor();
 $files = $extractor->extractAllFiles('path/to/file.kmz', 'extraction/directory');
 ```
 
-`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.
+`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:
+
+```php
+$files = $extractor->extractAllFiles('path/to/file.kmz');
+```
+
+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.
 
 ## Error handling
 
@@ -312,8 +318,14 @@ return [
         'http://earth.google.com/kml/2.0',
     ],
 
-    // Reserved for KMZ extraction. Not used yet.
+    // Where extractAllFiles() writes when given no destination.
+    // null means the system temp directory.
     'temp_directory' => null,
+
+    // Ceilings applied to a KMZ before anything is read out of it.
+    // Set either to 0 to disable it.
+    'max_archive_entries' => 5000,
+    'max_uncompressed_size' => 256 * 1024 * 1024,
 ];
 ```
 

config/kml-parser.php

@@ -36,9 +36,25 @@
     | Temporary Directory
     |--------------------------------------------------------------------------
     |
-    | This value determines the temporary directory used for extracting KMZ files.
-    | If null, the system temp directory will be used.
+    | Where KmzExtractor::extractAllFiles() writes when the caller names no
+    | destination. Each call gets its own directory underneath it. If null,
+    | the system temp directory is used.
     |
     */
     'temp_directory' => null,
+
+    /*
+    |--------------------------------------------------------------------------
+    | Archive Limits
+    |--------------------------------------------------------------------------
+    |
+    | A KMZ is a ZIP, and a ZIP can declare a handful of entries that expand
+    | into far more than the machine has. An archive breaching either ceiling
+    | is rejected before anything is read out of it. A real KMZ is a KML plus
+    | its icons, nowhere near either number. Set one to 0 to disable it.
+    |
+    */
+    'max_archive_entries' => 5000,
+
+    'max_uncompressed_size' => 256 * 1024 * 1024,
 ];

src/Exceptions/KmzExtractorException.php

@@ -23,4 +23,24 @@ public static function invalidZipFile(string $path): self
     {
         return new self("Invalid KMZ file: {$path}");
     }
+
+    public static function tooManyEntries(int $count, int $max): self
+    {
+        return new self("KMZ archive holds {$count} entries, more than the {$max} allowed");
+    }
+
+    public static function archiveTooLarge(int $max): self
+    {
+        return new self("KMZ archive expands to more than the {$max} bytes allowed");
+    }
+
+    public static function unsafeEntry(string $name): self
+    {
+        return new self("KMZ archive holds an entry that would escape the destination: {$name}");
+    }
+
+    public static function destinationNotWritable(string $path): self
+    {
+        return new self("Unable to create the extraction directory: {$path}");
+    }
 }

src/KmzExtractor.php

@@ -2,40 +2,45 @@
 
 namespace PlinCode\KmlParser;
 
-use PlinCode\KmlParser\Exceptions\KmlException;
 use PlinCode\KmlParser\Exceptions\KmzExtractorException;
+use PlinCode\KmlParser\Traits\ReadsPackageConfig;
 use ZipArchive;
 
 class KmzExtractor
 {
+    use ReadsPackageConfig;
+
+    /**
+     * Ceilings applied to an archive before anything is read out of it.
+     *
+     * A KMZ is a ZIP, and a ZIP can declare a handful of entries that expand
+     * into far more than the machine has. These are deliberately generous: a
+     * real KMZ is a KML plus its icons, nowhere near either limit.
+     */
+    public const DEFAULT_MAX_ENTRIES = 5000;
+
+    public const DEFAULT_MAX_UNCOMPRESSED_SIZE = 268435456; // 256 MB
+
     /**
      * Extract KML content from a KMZ file
+     *
+     * @throws KmzExtractorException
      */
     public function extractKmlContent(string $path): string
     {
-        if (! file_exists($path)) {
-            throw KmzExtractorException::fileNotFound($path);
-        }
-
-        $zip = new ZipArchive;
-        if ($zip->open($path) !== true) {
-            throw KmzExtractorException::invalidZipFile($path);
-        }
+        $zip = $this->open($path);
 
         try {
-            $kmlFiles = array_filter(
-                array_map(
-                    fn (int $i) => $zip->getNameIndex($i),
-                    range(0, $zip->numFiles - 1)
-                ),
-                fn (string $filename) => pathinfo($filename, PATHINFO_EXTENSION) === 'kml'
-            );
-
-            if (empty($kmlFiles)) {
+            $this->guardArchive($zip);
+
+            $kmlIndex = $this->firstKmlIndex($zip);
+
+            if ($kmlIndex === null) {
                 throw KmzExtractorException::noKmlFound();
             }
 
-            $kmlContent = $zip->getFromIndex(array_key_first($kmlFiles));
+            $kmlContent = $zip->getFromIndex($kmlIndex);
+
             if ($kmlContent === false) {
                 throw KmzExtractorException::failedToExtract('Failed to read KML file from archive');
             }
@@ -49,32 +54,150 @@ public function extractKmlContent(string $path): string
     /**
      * Extract all files from KMZ archive
      *
-     * @throws KmlException If the KMZ file cannot be found or opened
+     * Without a destination the files go to the configured temp_directory, or
+     * to the system temp directory, in a directory of their own.
+     *
+     * @return array<int, string> the entry names that were written
+     *
+     * @throws KmzExtractorException
      */
-    public function extractAllFiles(string $kmzPath, string $destination): array
+    public function extractAllFiles(string $kmzPath, ?string $destination = null): array
     {
-        if (! file_exists($kmzPath)) {
-            throw new KmlException("KMZ file not found: {$kmzPath}");
+        $destination ??= $this->defaultDestination();
+
+        $zip = $this->open($kmzPath);
+
+        try {
+            $this->guardArchive($zip);
+
+            /*
+             * The warning mkdir() raises carries less than the exception
+             * thrown below, and an application turning warnings into
+             * exceptions would otherwise get that one instead of ours. The
+             * return value is what is acted on, the second is_dir() covers
+             * another process winning the race.
+             */
+            if (! is_dir($destination) && ! @mkdir($destination, 0755, true) && ! is_dir($destination)) {
+                throw KmzExtractorException::destinationNotWritable($destination);
+            }
+
+            if (! $zip->extractTo($destination)) {
+                throw KmzExtractorException::failedToExtract("Unable to extract the archive into {$destination}");
+            }
+
+            return $this->entryNames($zip);
+        } finally {
+            $zip->close();
+        }
+    }
+
+    /**
+     * The directory extractAllFiles() writes to when the caller names none.
+     */
+    public function defaultDestination(): string
+    {
+        $configured = $this->packageConfig('kml-parser.temp_directory', null);
+        $base = is_string($configured) && $configured !== '' ? $configured : sys_get_temp_dir();
+
+        return rtrim($base, '/\\').DIRECTORY_SEPARATOR.'kml-parser-'.uniqid();
+    }
+
+    /**
+     * @throws KmzExtractorException
+     */
+    protected function open(string $path): ZipArchive
+    {
+        if (! file_exists($path)) {
+            throw KmzExtractorException::fileNotFound($path);
         }
 
         $zip = new ZipArchive;
-        if ($zip->open($kmzPath) !== true) {
-            throw new KmlException("Unable to open KMZ file: {$kmzPath}");
+
+        if ($zip->open($path) !== true) {
+            throw KmzExtractorException::invalidZipFile($path);
         }
 
-        if (! file_exists($destination)) {
-            mkdir($destination, 0755, true);
+        return $zip;
+    }
+
+    /**
+     * Reject an archive that is too large to trust before reading anything out
+     * of it, and reject any entry whose name would escape the destination.
+     *
+     * @throws KmzExtractorException
+     */
+    protected function guardArchive(ZipArchive $zip): void
+    {
+        $maxEntries = (int) $this->packageConfig('kml-parser.max_archive_entries', self::DEFAULT_MAX_ENTRIES);
+        $maxSize = (int) $this->packageConfig('kml-parser.max_uncompressed_size', self::DEFAULT_MAX_UNCOMPRESSED_SIZE);
+
+        if ($maxEntries > 0 && $zip->numFiles > $maxEntries) {
+            throw KmzExtractorException::tooManyEntries($zip->numFiles, $maxEntries);
         }
 
-        $zip->extractTo($destination);
+        $total = 0;
 
-        $extractedFiles = [];
         for ($i = 0; $i < $zip->numFiles; $i++) {
-            $extractedFiles[] = $zip->getNameIndex($i);
+            $stat = $zip->statIndex($i);
+
+            if ($stat === false) {
+                throw KmzExtractorException::failedToExtract("Unable to read entry {$i} of the archive");
+            }
+
+            $this->guardEntryName((string) $stat['name']);
+
+            $total += (int) $stat['size'];
+
+            if ($maxSize > 0 && $total > $maxSize) {
+                throw KmzExtractorException::archiveTooLarge($maxSize);
+            }
         }
+    }
 
-        $zip->close();
+    /**
+     * @throws KmzExtractorException
+     */
+    protected function guardEntryName(string $name): void
+    {
+        if (str_starts_with($name, '/') || str_starts_with($name, '\\') || preg_match('#^[A-Za-z]:[\\\\/]#', $name) === 1) {
+            throw KmzExtractorException::unsafeEntry($name);
+        }
+
+        foreach (preg_split('#[\\\\/]#', $name) ?: [] as $segment) {
+            if ($segment === '..') {
+                throw KmzExtractorException::unsafeEntry($name);
+            }
+        }
+    }
+
+    protected function firstKmlIndex(ZipArchive $zip): ?int
+    {
+        for ($i = 0; $i < $zip->numFiles; $i++) {
+            $name = $zip->getNameIndex($i);
+
+            if ($name !== false && strtolower(pathinfo($name, PATHINFO_EXTENSION)) === 'kml') {
+                return $i;
+            }
+        }
+
+        return null;
+    }
+
+    /**
+     * @return array<int, string>
+     */
+    protected function entryNames(ZipArchive $zip): array
+    {
+        $names = [];
+
+        for ($i = 0; $i < $zip->numFiles; $i++) {
+            $name = $zip->getNameIndex($i);
+
+            if ($name !== false) {
+                $names[] = $name;
+            }
+        }
 
-        return $extractedFiles;
+        return $names;
     }
 }