diff --git a/cve_bin_tool/extractor.py b/cve_bin_tool/extractor.py index 5dd53f1a08..f8e2a3aa67 100644 --- a/cve_bin_tool/extractor.py +++ b/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): diff --git a/test/assets/tarfile_abs_test.tar b/test/assets/tarfile_abs_test.tar new file mode 100644 index 0000000000..59c23d0c44 Binary files /dev/null and b/test/assets/tarfile_abs_test.tar differ diff --git a/test/test_extractor.py b/test/test_extractor.py index 6fd69b5d42..ebe834555c 100644 --- a/test/test_extractor.py +++ b/test/test_extractor.py @@ -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"""