#55 - UnzipTask: limit the number of entries and the uncompressed size

CHANGELOG.md

@@ -1,6 +1,9 @@
 Latest
 ------
 
+### Changes
+* [#55](https://github.com/cleverage/archive-process-bundle/issues/55) `UnzipTask` (`archive_unzip`): new `max_files` and `max_size` options (default `null`, no limit) to protect the server against "zip bombs": the number of entries and their total uncompressed size are checked before anything is extracted, the task fails with a `RuntimeException` when a limit is exceeded. Update documentation, add tests.
+
 v2.3
 ------
 

docs/reference/tasks/unzip_task.md

@@ -14,7 +14,7 @@ Accepted inputs
 ---------------
 
 `array` or `null`: the input array is merged with the task options, input values taking precedence over the
-configured ones. It may only contain the `filename` and/or `destination` keys (any other key throws an
+configured ones. It may only contain option keys (`filename`, `destination`, `max_files`, `max_size`; any other key throws an
 `UndefinedOptionsException`). A `null` input is handled as an empty array, so the options come from the task
 configuration only.
 
@@ -35,8 +35,10 @@ Options
 |---------------|----------|:--------:|---------|---------------------------------------------------------------------------------------------------|
 | `filename`    | `string` |  **X**   |         | Path of the zip archive to extract. It must exist and be readable (`\UnexpectedValueException`)   |
 | `destination` | `string` |  **X**   |         | Directory where the archive content is extracted                                                  |
+| `max_files`   | `?int`   |          | `null`  | Maximum number of entries of the archive (`null`: no limit)                                       |
+| `max_size`    | `?int`   |          | `null`  | Maximum total uncompressed size of the entries, in bytes (`null`: no limit)                       |
 
-Both options are required, but they can be provided either in the task configuration or in the input.
+`filename` and `destination` are required, but they can be provided either in the task configuration or in the input.
 They are validated when the task is executed, not at process initialization.
 
 Examples
@@ -84,3 +86,7 @@ Notes
 * Options are resolved on each execution of the task: when the task receives several inputs (e.g. after an iterable
   task), each input is extracted with its own `filename` and `destination`.
 * See the [Import the CSV files of an uploaded archive](../../cookbooks/import_archive.md) cookbook.
+* With an archive that is not trusted (received from a partner, uploaded by a user...), set `max_files` and `max_size`
+  to protect the server against "zip bombs": they are checked from the central directory of the archive before
+  anything is extracted, and the task fails with a `RuntimeException` when a limit is exceeded. An extraction failing
+  for another reason (e.g. a write error) may leave the entries already extracted in the destination directory.

src/Task/UnzipTask.php

@@ -35,7 +35,7 @@ public function execute(ProcessState $state): void
             throw new UnexpectedValueException(\sprintf('UnzipTask expects an array or null input, %s given', get_debug_type($state->getInput())));
         }
         /**
-         * @var array{filename: string, destination: string} $options
+         * @var array{filename: string, destination: string, max_files: ?int, max_size: ?int} $options
          */
         $options = $this->getOptions($state);
         $filename = $options['filename'];
@@ -56,6 +56,14 @@ public function execute(ProcessState $state): void
             throw new RuntimeException("Unable to open file {$filename} with code {$res}");
         }
 
+        try {
+            $this->checkLimits($zipArchive, $filename, $options['max_files'], $options['max_size']);
+        } catch (RuntimeException $e) {
+            $zipArchive->close();
+
+            throw $e;
+        }
+
         // The PHP warning is replaced by the exception below
         error_clear_last();
         $extracted = @$zipArchive->extractTo($dest);
@@ -68,15 +76,46 @@ public function execute(ProcessState $state): void
         $state->setOutput($dest);
     }
 
+    /**
+     * Checks the archive against the limits before extracting anything (protection against zip bombs): the number of
+     * entries and their uncompressed sizes are read from the central directory of the archive.
+     */
+    protected function checkLimits(\ZipArchive $zipArchive, string $filename, ?int $maxFiles, ?int $maxSize): void
+    {
+        if (null !== $maxFiles && $zipArchive->numFiles > $maxFiles) {
+            throw new RuntimeException("Unable to extract file {$filename}: it contains {$zipArchive->numFiles} entries, more than the max_files option ({$maxFiles})");
+        }
+        if (null === $maxSize) {
+            return;
+        }
+        $size = 0;
+        for ($index = 0; $index < $zipArchive->numFiles; ++$index) {
+            $stat = $zipArchive->statIndex($index);
+            if (false === $stat) {
+                throw new RuntimeException("Unable to extract file {$filename}: the entry {$index} cannot be read");
+            }
+            $size += $stat['size'];
+            if ($size > $maxSize) {
+                throw new RuntimeException("Unable to extract file {$filename}: its uncompressed size is more than the max_size option ({$maxSize} bytes)");
+            }
+        }
+    }
+
     protected function configureOptions(OptionsResolver $resolver): void
     {
         $resolver->setRequired(['filename', 'destination']);
         $resolver->setAllowedTypes('filename', ['string']);
         $resolver->setAllowedTypes('destination', ['string']);
+        $resolver->setDefaults([
+            'max_files' => null,
+            'max_size' => null,
+        ]);
+        $resolver->setAllowedTypes('max_files', ['null', 'int']);
+        $resolver->setAllowedTypes('max_size', ['null', 'int']);
     }
 
     /**
-     * @return array{filename: string, destination: string}|null
+     * @return array{filename: string, destination: string, max_files: ?int, max_size: ?int}|null
      */
     #[\Override]
     protected function getOptions(ProcessState $state): ?array

tests/Task/UnzipTaskTest.php

@@ -13,6 +13,7 @@
 
 namespace CleverAge\ArchiveProcessBundle\Tests\Task;
 
+use CleverAge\ArchiveProcessBundle\Exception\RuntimeException;
 use CleverAge\ArchiveProcessBundle\Task\UnzipTask;
 use CleverAge\ProcessBundle\Configuration\ProcessConfiguration;
 use CleverAge\ProcessBundle\Configuration\TaskConfiguration;
@@ -233,6 +234,54 @@ private function execute(UnzipTask $task, ProcessState $state, mixed $input): mi
         return $state->getOutput();
     }
 
+    public function testMaxFilesIsChecked(): void
+    {
+        [$task, $state] = $this->createTask([
+            'filename' => $this->dir.'/archive1.zip',
+            'destination' => $this->dir.'/destination',
+            'max_files' => 0,
+        ]);
+
+        try {
+            $this->execute($task, $state, null);
+            self::fail('The extraction should have failed');
+        } catch (RuntimeException $e) {
+            self::assertSame("Unable to extract file {$this->dir}/archive1.zip: it contains 1 entries, more than the max_files option (0)", $e->getMessage());
+        }
+        self::assertDirectoryDoesNotExist($this->dir.'/destination');
+    }
+
+    public function testMaxSizeIsChecked(): void
+    {
+        // "content of file1.txt": 20 bytes
+        [$task, $state] = $this->createTask([
+            'filename' => $this->dir.'/archive1.zip',
+            'destination' => $this->dir.'/destination',
+            'max_size' => 19,
+        ]);
+
+        try {
+            $this->execute($task, $state, null);
+            self::fail('The extraction should have failed');
+        } catch (RuntimeException $e) {
+            self::assertSame("Unable to extract file {$this->dir}/archive1.zip: its uncompressed size is more than the max_size option (19 bytes)", $e->getMessage());
+        }
+        self::assertDirectoryDoesNotExist($this->dir.'/destination');
+    }
+
+    public function testArchiveWithinTheLimitsIsExtracted(): void
+    {
+        [$task, $state] = $this->createTask([
+            'filename' => $this->dir.'/archive1.zip',
+            'destination' => $this->dir.'/destination',
+            'max_files' => 1,
+            'max_size' => 20,
+        ]);
+
+        self::assertSame($this->dir.'/destination', $this->execute($task, $state, null));
+        self::assertFileExists($this->dir.'/destination/file1.txt');
+    }
+
     private function createArchive(string $filename, string $entry): void
     {
         $zip = new \ZipArchive();