fix: allow extractraction on all files to fail (#1285)

* fixes #1281
* Related to #1181 

Changes in #1181 to check return codes made it so that cve-bin-tool failed if any file did not extract correctly. 

This changes the default so that failures during extraction are logged but do not halt the scan.  Error messages are not logged when the file extension is .exe because users found those confusing (cve-bin-tool tries to unzip all .exe files in case they are self-extracting zipfiles, but many are not)

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

Author: terriko

cve_bin_tool/async_utils.py

@@ -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,

cve_bin_tool/extractor.py

@@ -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:
+                    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:

test/assets/empty-file.exe

@@ -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)

test/assets/empty-file.zip

@@ -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__))

test/test_async_utils.py

@@ -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:
@@ -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)