Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: allow extractraction on all files to fail #1285

Merged
merged 11 commits into from
Aug 2, 2021
2 changes: 1 addition & 1 deletion cve_bin_tool/async_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def run_coroutine(coro):
return result


async def aio_run_command(args, process_can_fail=False):
async def aio_run_command(args, process_can_fail=True):
process = await asyncio.create_subprocess_exec(
*args,
stdout=asyncio.subprocess.PIPE,
Expand Down
14 changes: 11 additions & 3 deletions cve_bin_tool/extractor.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,17 +178,25 @@ async def extract_file_cab(filename, extraction_path):
return 0

@staticmethod
async def extract_file_zip(filename, extraction_path, process_can_fail=False):
async def extract_file_zip(filename, extraction_path, process_can_fail=True):
"""Extract zip files"""

is_exe = filename.endswith(".exe")
if await aio_inpath("unzip"):
stdout, stderr, _ = await aio_run_command(
["unzip", "-n", "-d", extraction_path, filename]
["unzip", "-n", "-d", extraction_path, filename], process_can_fail
)
if stderr or not stdout:
if is_exe:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return (int(not is_exe))
Take it or leave it, just saves a couple of lines, but not great readability.

return 0 # not all .exe files are zipfiles, no need for error
return 1
elif await aio_inpath("7z"):
stdout, stderr, _ = await aio_run_command(["7z", "x", filename])
stdout, stderr, _ = await aio_run_command(
["7z", "x", filename], process_can_fail
)
if stderr or not stdout:
if is_exe:
return 0 # not all .exe files are zipfiles, no need for error
return 1
else:
with ErrorHandler(mode=ErrorMode.Ignore) as e:
Expand Down
Empty file added test/assets/empty-file.exe
Empty file.
Empty file added test/assets/empty-file.zip
Empty file.
2 changes: 1 addition & 1 deletion test/test_async_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,4 @@ async def test_aio_run_command_success():
async def test_aio_run_command_returncode_non_zero():
with unittest.mock.patch("asyncio.create_subprocess_exec", new=mkexec(1)):
with pytest.raises(subprocess.CalledProcessError):
await aio_run_command(("echo", "hello"))
await aio_run_command(("echo", "hello"), process_can_fail=False)
19 changes: 19 additions & 0 deletions test/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,25 @@ def test_no_extraction(self):
"""Test scanner against curl-7.20.0 rpm with extraction turned off"""
assert main(["cve-bin-tool", os.path.join(self.tempdir, CURL_7_20_0_RPM)]) != 0

def test_extract_bad_zip_messages(self, caplog):
"""Test that bad zip files are logged as extraction failed, but
bad exe files produce no such message"""
BAD_EXE_FILE = os.path.join(
os.path.join(os.path.abspath(os.path.dirname(__file__)), "assets"),
"empty-file.exe",
)
with caplog.at_level(logging.WARNING):
main(["cve-bin-tool", BAD_EXE_FILE])
assert "Failure extracting" not in caplog.text

BAD_ZIP_FILE = os.path.join(
os.path.join(os.path.abspath(os.path.dirname(__file__)), "assets"),
"empty-file.zip",
)
with caplog.at_level(logging.WARNING):
main(["cve-bin-tool", BAD_ZIP_FILE])
assert "Failure extracting" in caplog.text

def test_exclude(self, caplog):
"""Test that the exclude paths are not scanned"""
test_path = os.path.abspath(os.path.dirname(__file__))
Expand Down
17 changes: 17 additions & 0 deletions test/test_extractor.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,14 @@
os.path.join(os.path.abspath(os.path.dirname(__file__)), "assets"),
"cab-test-python3.8.cab",
)
BAD_EXE_FILE = os.path.join(
os.path.join(os.path.abspath(os.path.dirname(__file__)), "assets"),
"empty-file.exe",
)
BAD_ZIP_FILE = os.path.join(
os.path.join(os.path.abspath(os.path.dirname(__file__)), "assets"),
"empty-file.zip",
)


class TestExtractorBase:
Expand Down Expand Up @@ -216,3 +224,12 @@ async def test_extract_file_zip(self):
]
):
assert os.path.isdir(path)

@pytest.mark.asyncio
async def test_bad_zip(self):
"""Test handling of invalid zip files. No errors should be raised.
Log messages differ for .exe and .zip and are tested in test_cli.py
"""

self.extract_files(BAD_EXE_FILE)
self.extract_files(BAD_ZIP_FILE)