Skip to content

Commit

Permalink
fix: add absolute tarfile test, windows workaround
Browse files Browse the repository at this point in the history
Signed-off-by: Terri Oda <[email protected]>
  • Loading branch information
terriko committed Feb 22, 2024
1 parent 34784dc commit 326adf6
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 3 deletions.
15 changes: 12 additions & 3 deletions cve_bin_tool/extractor.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,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
Expand All @@ -112,6 +120,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

Expand Down
Binary file added test/assets/tarfile_abs_test.tar
Binary file not shown.
12 changes: 12 additions & 0 deletions test/test_extractor.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,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"""
Expand Down

0 comments on commit 326adf6

Please sign in to comment.