CVE-2023-22898: Improper Input Validation (CWE-20)
value: workers/extractor.py in Pandora (aka pandora-analysis/pandora) 1.3.0 allows a denial of service when an attacker submits a deeply nested ZIP archive (aka ZIP bomb).
CWE description: The product receives input or data, but it does
        not validate or incorrectly validates that the input has the
        properties that are required to process the data safely and
        correctly.
CVSS v3 base score: 6.5
Published: 2023-01-10T02:15Z
Repository: https://github.com/pandora-analysis/pandora
Language: Python
Files changed: pandora/workers/extractor.py, pandora/workers/extractor.yml.sample
Commit date: 2023-01-04 13:51:03 +0100
Commit message: fix: Stop extractor on zip bomb (check total amount of extracted files and recursivity)

Diff: diff --git a/pandora/workers/extractor.py b/pandora/workers/extractor.py
index c6ad0d5..7b1d52e 100644
--- a/pandora/workers/extractor.py
+++ b/pandora/workers/extractor.py
@@ -499,10 +499,33 @@ class Extractor(BaseWorker):
         dfvfs_info = self.check_dfvfs(task.file, True)
         if not (task.file.is_archive or task.file.is_eml or task.file.is_msg or dfvfs_info):
             report.status = Status.NOTAPPLICABLE
             return
 
+        # Check if we reach the max recursivity (archive in archive in archive...)
+        _curtask = task
+        _cur_recurse = self.max_recurse
+        _cur_max_files_in_recurse = self.max_files_in_recursive_archive
+        while _cur_recurse > 0 and _cur_max_files_in_recurse > 0:
+            if not _curtask.parent:
+                break
+            _curtask = _curtask.parent
+            _cur_recurse -= 1
+            _cur_max_files_in_recurse -= len(_curtask.extracted)
+
+        if _cur_recurse <= 0:
+            self.logger.warning(f'File {task.file.path.name} is too deep in the recursion chain (>{self.max_recurse}).')
+            report.status = Status.ERROR if self.max_is_error else Status.ALERT
+            report.add_details('Warning', f'File {task.file.path.name} is too deep in the recursion chain (>{self.max_recurse}). If you want to scan it anyway, click on Actions > Rescan file.')
+            return
+
+        if _cur_max_files_in_recurse <= 0:
+            self.logger.warning(f'File {task.file.path.name} cannot be extracted, too many files (>{self.max_files_in_recursive_archive}) in the recursive archive.')
+            report.status = Status.ERROR if self.max_is_error else Status.ALERT
+            report.add_details('Warning', f'File {task.file.path.name} cannot be extracted, too many files (>{self.max_files_in_recursive_archive}) in the recursive archive. If you want to scan it anyway, click on Actions > Rescan file.')
+            return
+
         if not task.user:
             raise PandoraException('The task user is missing. Should not happen, but investigate if it does.')
 
         pandora = Pandora()
 
@@ -597,17 +620,20 @@ class Extractor(BaseWorker):
             pandora.add_extracted_reference(task, new_task)
             pandora.enqueue_task(new_task)
             tasks.append(new_task)
 
         shutil.rmtree(extracted_dir)
-        # wait for all the tasks to finish
-        while not all(t.workers_done for t in tasks):
-            time.sleep(1)
-
-        if tasks:
-            report.status = max(t.status for t in tasks)
-            if report.status > Status.CLEAN:
-                report.add_details('Warning', 'There are suspicious files in this archive, click on the "Extracted" tab for more.')
-        elif not report.status == Status.NOTAPPLICABLE:
+
+        if not tasks and not report.status == Status.NOTAPPLICABLE:
             # Nothing was extracted
             report.status = Status.WARN
             report.add_details('Warning', 'Looks like the archive is empty (?). This is suspicious.')
+        elif report.status not in [Status.ERROR, Status.WARN, Status.ALERT, Status.OVERWRITE]:
+            # wait for all the workers to finish, or have one of them raising an ALERT
+            while not all(t.workers_done for t in tasks):
+                for t in tasks:
+                    # If any of the task is marked as ALERT or OVERWRITE, we can quit.
+                    if t.workers_done and t.status >= Status.ALERT:
+                        report.add_details('Warning', 'There are suspicious files in this archive, click on the "Extracted" tab for more.')
+                        break
+                time.sleep(1)
+            report.status = max(t.status for t in tasks if t.workers_done)
diff --git a/pandora/workers/extractor.yml.sample b/pandora/workers/extractor.yml.sample
index 8b56bd9..053ce2c 100644
--- a/pandora/workers/extractor.yml.sample
+++ b/pandora/workers/extractor.yml.sample
@@ -3,10 +3,12 @@ meta:
   description: Extracts content from the uploaded object (email, archive)
   replicas: 5
 
 settings:
   max_files_in_archive: 15
+  max_files_in_recursive_archive: 25
+  max_recurse: 2
   max_extracted_filesize_in_mb: 100
   max_is_error: no
   zip_passwords:
     - ''
     - virus

Vulnerable code:         # wait for all the tasks to finish
        while not all(t.workers_done for t in tasks):
            time.sleep(1)

        if tasks:
            report.status = max(t.status for t in tasks)
            if report.status > Status.CLEAN:
                report.add_details('Warning', 'There are suspicious files in this archive, click on the "Extracted" tab for more.')
        elif not report.status == Status.NOTAPPLICABLE:
Fixed code:         # Check if we reach the max recursivity (archive in archive in archive...)
        _curtask = task
        _cur_recurse = self.max_recurse
        _cur_max_files_in_recurse = self.max_files_in_recursive_archive
        while _cur_recurse > 0 and _cur_max_files_in_recurse > 0:
            if not _curtask.parent:
                break
            _curtask = _curtask.parent
            _cur_recurse -= 1
            _cur_max_files_in_recurse -= len(_curtask.extracted)

        if _cur_recurse <= 0:
            self.logger.warning(f'File {task.file.path.name} is too deep in the recursion chain (>{self.max_recurse}).')
            report.status = Status.ERROR if self.max_is_error else Status.ALERT
            report.add_details('Warning', f'File {task.file.path.name} is too deep in the recursion chain (>{self.max_recurse}). If you want to scan it anyway, click on Actions > Rescan file.')
            return

        if _cur_max_files_in_recurse <= 0:
            self.logger.warning(f'File {task.file.path.name} cannot be extracted, too many files (>{self.max_files_in_recursive_archive}) in the recursive archive.')
            report.status = Status.ERROR if self.max_is_error else Status.ALERT
            report.add_details('Warning', f'File {task.file.path.name} cannot be extracted, too many files (>{self.max_files_in_recursive_archive}) in the recursive archive. If you want to scan it anyway, click on Actions > Rescan file.')
            return


        if not tasks and not report.status == Status.NOTAPPLICABLE:
        elif report.status not in [Status.ERROR, Status.WARN, Status.ALERT, Status.OVERWRITE]:
            # wait for all the workers to finish, or have one of them raising an ALERT
            while not all(t.workers_done for t in tasks):
                for t in tasks:
                    # If any of the task is marked as ALERT or OVERWRITE, we can quit.
                    if t.workers_done and t.status >= Status.ALERT:
                        report.add_details('Warning', 'There are suspicious files in this archive, click on the "Extracted" tab for more.')
                        break
                time.sleep(1)
            report.status = max(t.status for t in tasks if t.workers_done)
  max_files_in_recursive_archive: 25
  max_recurse: 2