fix: use tarfile extract filters to open tarfiles more safely (#3769)

Also changed pre-commit config so interrogate is at the top and its output
doesn't obscure more urgent error messages.

Note that this is still missing a test and appropriate windows behaviour; those will be coming in future PRs.

Signed-off-by: Terri Oda <[email protected]>

Author: terriko

.github/workflows/testing.yml

@@ -210,6 +210,8 @@ jobs:
       - uses: technote-space/get-diff-action@f27caffdd0fb9b13f4fc191c016bb4e0632844af # v6.1.2
         with:
           PATTERNS: |
+            cve_bin_tool/*.py
+            cve_bin_tool/data_sources/*.py
             cve_bin_tool/checkers/*.py
             test/condensed-downloads/*
           FILES: |

.pre-commit-config.yaml

@@ -1,4 +1,12 @@
 repos:
+-   repo: https://github.com/econchick/interrogate
+    rev: 1.5.0
+    hooks:
+      - id: interrogate
+        verbose: True
+        exclude: ^(locales|presentation|fuzz|test|cve_bin_tool/checkers|build)
+        args: ["-vv", "-i", "-I", "-M", "-C", "-n", "-p", "-f", "60.0"]
+
 -   repo: https://github.com/pycqa/isort
     rev: 5.13.2
     hooks:
@@ -71,11 +79,4 @@ repos:
             test/utils.py|
         )$
 
--   repo: https://github.com/econchick/interrogate
-    rev: 1.5.0
-    hooks:
-      - id: interrogate
-        verbose: True
-        exclude: ^(locales|presentation|fuzz|test|cve_bin_tool/checkers|build)
-        args: ["-vv", "-i", "-I", "-M", "-C", "-n", "-p", "-f", "60.0"]
 

cve_bin_tool/extractor.py

@@ -8,6 +8,7 @@
 import re
 import shutil
 import sys
+import tarfile
 import tempfile
 from pathlib import Path
 
@@ -73,11 +74,42 @@ def can_extract(self, filename):
                 return True
         return False
 
-    @staticmethod
-    async def extract_file_tar(filename, extraction_path):
+    def tar_member_filter(self, members, extraction_path):
+        """Generator function to serve as a backported filter for tarfile extraction
+        based on https://docs.python.org/3/library/tarfile.html#examples
+        """
+        for tarmember in members:
+            if tarmember.isfile() and str(
+                Path(extraction_path, tarmember.name).resolve()
+            ).startsWith(extraction_path):
+                yield tarmember
+
+    async def extract_file_tar(self, filename, extraction_path):
         """Extract tar files"""
+
+        # make sure we have full path for later checks
+        extraction_path = str(Path(extraction_path).resolve())
         with ErrorHandler(mode=ErrorMode.Ignore) as e:
-            await aio_unpack_archive(filename, extraction_path)
+            tar = tarfile.open(filename)
+            # Python 3.12 has a data filter we can use in extract
+            # tarfile has this available in older versions as well
+            if hasattr(tarfile, "data_filter"):
+                tar.extractall(path=extraction_path, filter="data")  # nosec
+                # nosec line because bandit doesn't understand filters yet
+
+            # FIXME: the backported fix is not working on windows.
+            # this leaves the current (unsafe) behaviour so we can fix at least one OS for now
+            elif sys.platform == "win32":
+                tar.extractall(path=extraction_path)  # nosec
+
+            # Some versions may need us to implement a filter to avoid unsafe behaviour
+            # we could consider logging a warning here
+            else:
+                tar.extractall(
+                    path=extraction_path,
+                    members=self.tar_member_filter(tar, extraction_path),
+                )  # nosec
+            tar.close()
         return e.exit_code
 
     async def extract_file_rpm(self, filename, extraction_path):