fix: add tarfile test, minor tarfile refactor (#3962)

This adds a tarfile with an absolute path file in it for testing and switches our file open to use a with construction. (This helps with some experiments we're doing to improve the safety of tarfile extraction in windows.)

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

Author: terriko

cve_bin_tool/extractor.py

@@ -131,26 +131,27 @@ async def extract_file_tar(self, filename, extraction_path):
         # make sure we have full path for later checks
         extraction_path = str(Path(extraction_path).resolve())
         with ErrorHandler(mode=ErrorMode.Ignore) as e:
-            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
+                with tarfile.open(filename) as tar:
+                    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
+                # use unsafe extraction for now, fix will come in separate PR
+                with tarfile.open(filename) as tar:
+                    tar.extractall(path=extraction_path)  # nosec - fix in progress
 
             # 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()
+                with tarfile.open(filename) as tar:
+                    tar.extractall(
+                        path=extraction_path,
+                        members=self.tar_member_filter(tar, extraction_path),
+                    )  # nosec
+
         return e.exit_code
 
     async def extract_file_rpm(self, filename, extraction_path):

test/assets/tarfile_abs_test.tar

@@ -116,6 +116,18 @@ async def test_extract_file_tar(self, extension_list: list[str]):
         ):
             assert Path(dir_path).is_dir()
 
+    @pytest.mark.asyncio
+    async def test_extract_file_tar_absolute(self):
+        """Test against a tarfile with absolute file names.
+        It should not extract to /tmp/cve-bin-tool_tarfile_test.txt"""
+
+        abs_tar_test = (
+            Path(__file__).parent.resolve() / "assets" / "tarfile_abs_test.tar"
+        )
+        self.extract_files(abs_tar_test)
+        assert not Path("/tmp/cve-bin-tool_tarfile_abs_test.txt").is_file()  # nosec
+        # Bandit note: intentional hard-coded value for this test of absolute file extraction
+
     @pytest.mark.asyncio
     async def test_extract_cleanup(self):
         """Make sure tar extractor cleans up after itself"""