From 254ccbb45560e694eda3f0bbe50f647aa6b5df32 Mon Sep 17 00:00:00 2001 From: Terri Oda Date: Wed, 21 Feb 2024 16:11:27 -0800 Subject: [PATCH 01/10] fix: add absolute tarfile test, windows workaround Signed-off-by: Terri Oda --- cve_bin_tool/extractor.py | 15 ++++++++++++--- test/assets/tarfile_abs_test.tar | Bin 0 -> 10240 bytes test/test_extractor.py | 12 ++++++++++++ 3 files changed, 24 insertions(+), 3 deletions(-) create mode 100644 test/assets/tarfile_abs_test.tar diff --git a/cve_bin_tool/extractor.py b/cve_bin_tool/extractor.py index 5dd53f1a08..1267ad4610 100644 --- a/cve_bin_tool/extractor.py +++ b/cve_bin_tool/extractor.py @@ -138,10 +138,18 @@ async def extract_file_tar(self, filename, extraction_path): 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 + # backported path filter (below) doesn't work on windwos for some reason, + # use external tar if available + if await aio_inpath("tar"): + return aio_run_command( + ["tar", "-C", extraction_path, "-axf", filename] + ) + else: + # Fail and suggest upgrade to python 3.12 + self.logger.error( + f"Unable to extract {filename} safely, please upgrade to python 3.12" + ) # Some versions may need us to implement a filter to avoid unsafe behaviour # we could consider logging a warning here @@ -150,6 +158,7 @@ async def extract_file_tar(self, filename, extraction_path): path=extraction_path, members=self.tar_member_filter(tar, extraction_path), ) # nosec + tar.close() return e.exit_code diff --git a/test/assets/tarfile_abs_test.tar b/test/assets/tarfile_abs_test.tar new file mode 100644 index 0000000000000000000000000000000000000000..59c23d0c44fc4e33414c0d142c22caaadd97447b GIT binary patch literal 10240 zcmeIuK@P$o5QSlm!UbxfLVFz28cay4H82{FU%GQuP3rQW%`D!$5WZG!kM{D4&6Mtq zEXy2Z%-1x>Fis^%EOL^!s6%7w^v Date: Wed, 21 Feb 2024 16:31:22 -0800 Subject: [PATCH 02/10] chore: fix comment typo --- cve_bin_tool/extractor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cve_bin_tool/extractor.py b/cve_bin_tool/extractor.py index 1267ad4610..16a23cc43f 100644 --- a/cve_bin_tool/extractor.py +++ b/cve_bin_tool/extractor.py @@ -139,7 +139,7 @@ async def extract_file_tar(self, filename, extraction_path): # nosec line because bandit doesn't understand filters yet elif sys.platform == "win32": - # backported path filter (below) doesn't work on windwos for some reason, + # backported path filter (below) doesn't work on windows, # use external tar if available if await aio_inpath("tar"): return aio_run_command( From ca9cf3ea896872b6b7a974f24a2ea4bfb8c3f77c Mon Sep 17 00:00:00 2001 From: Terri Oda Date: Tue, 27 Feb 2024 13:30:59 -0800 Subject: [PATCH 03/10] fix: attempt tar call differently --- cve_bin_tool/extractor.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cve_bin_tool/extractor.py b/cve_bin_tool/extractor.py index 16a23cc43f..fe3a766855 100644 --- a/cve_bin_tool/extractor.py +++ b/cve_bin_tool/extractor.py @@ -141,11 +141,11 @@ async def extract_file_tar(self, filename, extraction_path): elif sys.platform == "win32": # backported path filter (below) doesn't work on windows, # use external tar if available - if await aio_inpath("tar"): + try: return aio_run_command( ["tar", "-C", extraction_path, "-axf", filename] ) - else: + except FileNotFoundError: # Fail and suggest upgrade to python 3.12 self.logger.error( f"Unable to extract {filename} safely, please upgrade to python 3.12" From b9638d4fffb12c21f0c95740cb1c844e927e1ee2 Mon Sep 17 00:00:00 2001 From: Terri Oda Date: Tue, 27 Feb 2024 14:43:21 -0800 Subject: [PATCH 04/10] fix: use 7z instead --- cve_bin_tool/extractor.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/cve_bin_tool/extractor.py b/cve_bin_tool/extractor.py index fe3a766855..1d393b41a7 100644 --- a/cve_bin_tool/extractor.py +++ b/cve_bin_tool/extractor.py @@ -140,12 +140,15 @@ async def extract_file_tar(self, filename, extraction_path): elif sys.platform == "win32": # backported path filter (below) doesn't work on windows, - # use external tar if available - try: - return aio_run_command( - ["tar", "-C", extraction_path, "-axf", filename] + # use external 7z if available + if await aio_inpath("7z"): + stdout, stderr, _ = await aio_run_command( + ["7z", "x", filename, f"-o{extraction_path}"] ) - except FileNotFoundError: + if not stdout: + return 1 + return 0 + else: # Fail and suggest upgrade to python 3.12 self.logger.error( f"Unable to extract {filename} safely, please upgrade to python 3.12" From b36fc057eb72ed15f10a7a1867eac8e4250c9ae7 Mon Sep 17 00:00:00 2001 From: Terri Oda Date: Thu, 14 Mar 2024 13:53:24 -0700 Subject: [PATCH 05/10] fix: try tar and 7z on windows --- cve_bin_tool/extractor.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/cve_bin_tool/extractor.py b/cve_bin_tool/extractor.py index 1d393b41a7..40f69ca655 100644 --- a/cve_bin_tool/extractor.py +++ b/cve_bin_tool/extractor.py @@ -140,14 +140,21 @@ async def extract_file_tar(self, filename, extraction_path): elif sys.platform == "win32": # backported path filter (below) doesn't work on windows, - # use external 7z if available - if await aio_inpath("7z"): + # use external tar or 7z if available + if await aio_inpath("tar"): + # windows tar (bsdtar) removes / by default stdout, stderr, _ = await aio_run_command( - ["7z", "x", filename, f"-o{extraction_path}"] + ["tar", "-x", "-C", extraction_path, "-f", filename] ) - if not stdout: - return 1 - return 0 + if stderr: + self.logger.error("**********Extraction failed: {stderr}") + elif await aio_inpath("7z"): + # `7z e` is extract without full paths + stdout, stderr, _ = await aio_run_command( + ["7z", "e", filename, f"-o{extraction_path}"] + ) + if stderr: + self.logger.error("**********Extraction failed: {stderr}") else: # Fail and suggest upgrade to python 3.12 self.logger.error( From eef655cf23fc1516ad4ca01567caa6e3d62c3cff Mon Sep 17 00:00:00 2001 From: Terri Oda Date: Mon, 18 Mar 2024 14:05:36 -0700 Subject: [PATCH 06/10] fix: move tarfile open/close with extract --- cve_bin_tool/extractor.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cve_bin_tool/extractor.py b/cve_bin_tool/extractor.py index 40f69ca655..98ba4f6301 100644 --- a/cve_bin_tool/extractor.py +++ b/cve_bin_tool/extractor.py @@ -131,12 +131,13 @@ 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 = tarfile.open(filename) tar.extractall(path=extraction_path, filter="data") # nosec # nosec line because bandit doesn't understand filters yet + tar.close() elif sys.platform == "win32": # backported path filter (below) doesn't work on windows, @@ -164,12 +165,13 @@ async def extract_file_tar(self, filename, extraction_path): # Some versions may need us to implement a filter to avoid unsafe behaviour # we could consider logging a warning here else: + tar = tarfile.open(filename) tar.extractall( path=extraction_path, members=self.tar_member_filter(tar, extraction_path), ) # nosec + tar.close() - tar.close() return e.exit_code async def extract_file_rpm(self, filename, extraction_path): From 19ea20e13a6bdc87842446397290736e259f51bc Mon Sep 17 00:00:00 2001 From: Terri Oda Date: Tue, 19 Mar 2024 12:02:36 -0700 Subject: [PATCH 07/10] ci: up windows timeouts --- .github/workflows/testing.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/testing.yml b/.github/workflows/testing.yml index 4907a8e489..28809cab0f 100644 --- a/.github/workflows/testing.yml +++ b/.github/workflows/testing.yml @@ -389,7 +389,7 @@ jobs: ) ) runs-on: windows-latest - timeout-minutes: 90 + timeout-minutes: 120 env: NO_EXIT_CVE_NUM: 1 PYTHONIOENCODING: 'utf8' @@ -467,7 +467,7 @@ jobs: ) ) runs-on: windows-latest - timeout-minutes: 120 + timeout-minutes: 150 env: LONG_TESTS: 1 NO_EXIT_CVE_NUM: 1 From 69853d84e4f3c0851315a0c39c354c590c4471a7 Mon Sep 17 00:00:00 2001 From: Terri Oda Date: Tue, 19 Mar 2024 15:51:32 -0700 Subject: [PATCH 08/10] fix: use with construction for tarfile open --- cve_bin_tool/extractor.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/cve_bin_tool/extractor.py b/cve_bin_tool/extractor.py index 98ba4f6301..7bda29aa48 100644 --- a/cve_bin_tool/extractor.py +++ b/cve_bin_tool/extractor.py @@ -134,10 +134,9 @@ async def extract_file_tar(self, filename, extraction_path): # 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 = tarfile.open(filename) - 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 - tar.close() elif sys.platform == "win32": # backported path filter (below) doesn't work on windows, @@ -165,12 +164,11 @@ async def extract_file_tar(self, filename, extraction_path): # Some versions may need us to implement a filter to avoid unsafe behaviour # we could consider logging a warning here else: - tar = tarfile.open(filename) - 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 From 3ed24997d1161321febf58fc93b04855a8b9244a Mon Sep 17 00:00:00 2001 From: Terri Oda Date: Wed, 20 Mar 2024 12:16:36 -0700 Subject: [PATCH 09/10] fix: revert windows workaround for now --- cve_bin_tool/extractor.py | 24 +++--------------------- 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/cve_bin_tool/extractor.py b/cve_bin_tool/extractor.py index 7bda29aa48..f8e2a3aa67 100644 --- a/cve_bin_tool/extractor.py +++ b/cve_bin_tool/extractor.py @@ -139,27 +139,9 @@ async def extract_file_tar(self, filename, extraction_path): # nosec line because bandit doesn't understand filters yet elif sys.platform == "win32": - # backported path filter (below) doesn't work on windows, - # use external tar or 7z if available - if await aio_inpath("tar"): - # windows tar (bsdtar) removes / by default - stdout, stderr, _ = await aio_run_command( - ["tar", "-x", "-C", extraction_path, "-f", filename] - ) - if stderr: - self.logger.error("**********Extraction failed: {stderr}") - elif await aio_inpath("7z"): - # `7z e` is extract without full paths - stdout, stderr, _ = await aio_run_command( - ["7z", "e", filename, f"-o{extraction_path}"] - ) - if stderr: - self.logger.error("**********Extraction failed: {stderr}") - else: - # Fail and suggest upgrade to python 3.12 - self.logger.error( - f"Unable to extract {filename} safely, please upgrade to python 3.12" - ) + # 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 From f452bbe1d7888cbe90af17eff766c5d430783ee2 Mon Sep 17 00:00:00 2001 From: Terri Oda Date: Wed, 20 Mar 2024 12:25:46 -0700 Subject: [PATCH 10/10] ci: revert changes --- .github/workflows/testing.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/testing.yml b/.github/workflows/testing.yml index 28809cab0f..4907a8e489 100644 --- a/.github/workflows/testing.yml +++ b/.github/workflows/testing.yml @@ -389,7 +389,7 @@ jobs: ) ) runs-on: windows-latest - timeout-minutes: 120 + timeout-minutes: 90 env: NO_EXIT_CVE_NUM: 1 PYTHONIOENCODING: 'utf8' @@ -467,7 +467,7 @@ jobs: ) ) runs-on: windows-latest - timeout-minutes: 150 + timeout-minutes: 120 env: LONG_TESTS: 1 NO_EXIT_CVE_NUM: 1