fix: Support detecting multiple product versions (#4911)

This patch refactors the core version checker code to support detecting
multiple versions of the same product in a single file. There was some
code already present that indicated an earlier intention to make this
work, but the version checker logic was written to exit early after
matching one version in a given file.

While doing this refactor, I also discovered that the version filename
parser test suite was broken, allowing faulty checkers and faulty
checker test cases to pass. I fixed the broken test, then Shan and I
fixed the handful of checkers that were found to be defective.

Roughly exhaustive list of changes:
* Rename `Checker.get_version()` to `get_versions()` to reflect its new
  abilities. Replace the name across code and documentation.
* Create a new `VersionMatchInfo` class to capture the return from
  `get_versions()`. For a given file, the checker will now report
whether it matched on the filename, the contents, or both, as well as a
list of all version numbers detected.
* Add some type hints! `get_versions()` is now explicit about what it returns.
  Previously, the intent was to return either a dict or a list of dicts,
and the caller had to apply some fragile logic to figure out which.
* Fix the test bug caused by that fragile logic.
  `TestCheckerVersionParser::test_filename_is` was failing to assert
anything when the checker returned an empty dict, and therefore was
allowing bad test cases and bad checkers alike to pass the test.
* After fixing that test, also fix the handful of filename version
  checkers that were broken all along but flying under the radar.
* Update `regex_find` in `util.py` to exhaust `pattern.finditer()`
  instead of exiting after finding one result with `pattern.search()`.
Return a list of strings instead of a string.
* Tweak the ignore checker logic to do a true regex search for the
  ignore pattern instead of some sketchy `str(a) in str(b)` stuff. The
ignore patterns are a `list[Pattern[str]]` so we should probably be
doing actual regex lookups here.

Fixes #4184.

---------

Signed-off-by: Steve Miller <[email protected]>
Co-authored-by: Shan Lee <[email protected]>

Author: stvml

cve_bin_tool/checkers/README.md

@@ -311,7 +311,7 @@ That regex might look like this: `3\?Xiph.Org libVorbis ([0-9]+\.[0-9]+\.[0-9]+)
 
 > If you can't get a signature match using just regex you may end up needing to
 > overwrite the
-> [`get_version()`](https://github.com/intel/cve-bin-tool/blob/main/cve_bin_tool/checkers/__init__.py#L120-L132)
+> [`get_versions()`](https://github.com/intel/cve-bin-tool/blob/main/cve_bin_tool/checkers/__init__.py#L120-L132)
 > method for the checker, but that should be a last resort if you can't find a
 > regex that works for `VERSION_PATTERNS`.
 >
@@ -322,10 +322,10 @@ That regex might look like this: `3\?Xiph.Org libVorbis ([0-9]+\.[0-9]+\.[0-9]+)
 > 1.3.7. While this was a nice example for how one might find a signature, it
 > in the end is not all the work that is needed to create a checker for
 > libvorbis. We need to make sure that any checker we develop has a
-> `get_version()` function which works for versions of the software which have
+> `get_versions()` function which works for versions of the software which have
 > CVEs. If not overridden in a subclass the Checker base class implements a
-> `get_version()` method which will use regex to determine the version (as
-> described above). In the case of libvorbis a custom `get_version()` function
+> `get_versions()` method which will use regex to determine the version (as
+> described above). In the case of libvorbis a custom `get_versions()` function
 > is likely needed, this is because the signature we found is not in the 1.2.0
 > version, where the CVE is found.
 
@@ -503,25 +503,26 @@ You can set an arbitrary number of workers. A good rule of thumb is to specify n
 
 The CVE-bin-checker works by extracting strings from binaries and determining
 if a given library has been compiled into the binary. For this, Checker class
-contains two methods: 1) `guess_contains()` and 2) `get_version()`.
+contains two methods: 1) `guess_contains()` and 2) `get_versions()`.
 
 1. `guess_contains()` method takes list of extracted string lines as an input and
 return True if it finds any of the `CONTAINS_PATTERNS` on any line from the
 lines.
-2. `get_version()` method takes list of extracted string lines and the filename as
-inputs and returns information about whether the binary contains the library
-in question, is a copy of the library in question, and if either of those are
-true it also returns a version string. If the binary does not contain the
-library, this function returns an empty dictionary.
+2. `get_versions()` method takes a list of extracted string lines and the filename as
+inputs and returns a VersionMatchInfo. This object contains whether a match was
+found in the filename, whether a match was found in the contents of the file, and a list
+of version strings found in the file. If no matches are found, both booleans will be false 
+and the list of version strings will be empty. If a match is found but no valid version 
+strings are matched, the list of version strings will contain only "UNKNOWN".
 
-If `curl` product is being scanned, `get_version()` method of CurlChecker will
-return following dictionary.
+For example, if the `curl` product is being scanned, the `get_versions()` method of CurlChecker will
+return the following object:
 
 ```json
 {
-  "is_or_contains": "is",
-  "modulename": "curl",
-  "version": "6.41.0"
+  "matched_filename": true,
+  "matched_contains": true,
+  "versions": ["6.41.0"]
 }
 ```
 

cve_bin_tool/checkers/__init__.py

@@ -9,7 +9,7 @@
 import sys
 
 from cve_bin_tool.error_handler import InvalidCheckerError
-from cve_bin_tool.util import regex_find
+from cve_bin_tool.util import VersionMatchInfo, regex_find
 
 if sys.version_info >= (3, 10):
     from importlib import metadata as importlib_metadata
@@ -475,21 +475,22 @@ def guess_contains(self, lines):
             return True
         return False
 
-    def get_version(self, lines, filename):
-        version_info = dict()
+    def get_versions(self, lines: str, filename: str) -> VersionMatchInfo:
+
+        version_details = VersionMatchInfo()
 
         if any(pattern.match(filename) for pattern in self.FILENAME_PATTERNS):
-            version_info["is_or_contains"] = "is"
+            version_details.matched_filename = True
 
-        if "is_or_contains" not in version_info and self.guess_contains(lines):
-            version_info["is_or_contains"] = "contains"
+        if self.guess_contains(lines):
+            version_details.matched_contains = True
 
-        if "is_or_contains" in version_info:
-            version_info["version"] = regex_find(
+        if version_details.matched_filename or version_details.matched_contains:
+            version_details.versions = regex_find(
                 lines, self.VERSION_PATTERNS, self.IGNORE_PATTERNS
             )
 
-        return version_info
+        return version_details
 
 
 BUILTIN_CHECKERS = {

cve_bin_tool/checkers/bind.py

@@ -28,6 +28,7 @@ class BindChecker(Checker):
         r"libisccc([-_]?(\d+\.)+\d.*)?\.so",
         r"libisccfg([-_]?(\d+\.)+\d.*)?\.so",
         r"libns([-_]?(\d+\.)+\d.*)?\.so",
+        r"libbind9([-_]?(\d+\.)+\d.*)?\.so",
     ]
     VERSION_PATTERNS = [
         r"version: BIND ([0-9]+\.[0-9]+\.[0-9]+)",  # for .rpm, .tgz, etc.

cve_bin_tool/checkers/curl.py

@@ -13,6 +13,7 @@
 
 Note: Some of the "first vulnerable in" data may not be entered correctly.
 """
+
 from cve_bin_tool.checkers import Checker
 
 
@@ -26,6 +27,8 @@ class CurlChecker(Checker):
         # r"error retrieving curl library information",
         # r"ignoring --proxy-capath, not supported by libcurl",
     ]
-    FILENAME_PATTERNS = [r"curl"]
+    FILENAME_PATTERNS = [
+        r"curl",
+    ]
     VERSION_PATTERNS = [r"\r?\ncurl[ -/]([678]+\.[0-9]+\.[0-9]+)"]
     VENDOR_PRODUCT = [("haxx", "curl")]

cve_bin_tool/checkers/ffmpeg.py

@@ -11,6 +11,7 @@
 
 Note: Some of the "first vulnerable in" data may not be entered correctly.
 """
+
 from cve_bin_tool.checkers import Checker
 
 
@@ -20,7 +21,12 @@ class FfmpegChecker(Checker):
         r"Codec '%s' is not recognized by FFmpeg.",
         r"Codec '%s' is known to FFmpeg, but no %s for it are available.",
     ]
-    FILENAME_PATTERNS = [r"ffmpeg"]
+    FILENAME_PATTERNS = [
+        # check if it's the library instead of the command line util
+        r"libffmpeg\.so(\.\d+)?",
+        # command line util
+        r"ffmpeg",
+    ]
     VERSION_PATTERNS = [
         r"%s version ([0-9]+\.[0-9]+\.[0-9]+)[a-zA-Z0-9 \(\)%~\-\r\n]*(?:avutil|FFmpeg)",
         r"FFmpeg version n?([0-9]+\.[0-9]+\.[0-9]+)",

cve_bin_tool/checkers/gstreamer.py

@@ -8,14 +8,20 @@
 https://www.cvedetails.com/vulnerability-list/vendor_id-9481/Gstreamer.html
 https://www.cvedetails.com/product/35669/Gstreamer-Project-Gstreamer.html?vendor_id=16047
 """
+
 from cve_bin_tool.checkers import Checker
 
 
 class GstreamerChecker(Checker):
     CONTAINS_PATTERNS = [
         r"http://bugzilla.gnome.org/enter_bug.cgi?product=GStreamer",
     ]
-    FILENAME_PATTERNS = [r"gstreamer"]
+    FILENAME_PATTERNS = [
+        # check if it's the library instead of the command line util
+        r"libgstreamer\.so(\.\d+)?",
+        # command line util
+        r"gstreamer",
+    ]
     VERSION_PATTERNS = [
         r"((\d+\.)+\d+)[a-zA-Z \r\n]*GStreamer ",
         r"gstreamer[a-zA-Z \r\n]+((\d+\.)+\d+)",

cve_bin_tool/checkers/icu.py

@@ -9,6 +9,7 @@
 https://www.cvedetails.com/vulnerability-list/vendor_id-17477/Icu-project.html
 https://www.cvedetails.com/product/101097/Unicode-International-Components-For-Unicode.html?vendor_id=21486
 """
+
 from cve_bin_tool.checkers import Checker
 
 
@@ -20,7 +21,13 @@ class IcuChecker(Checker):
         r"is about 300kB larger than the ucadata-implicithan\.icu version\.",
         r"the ucadata-unihan\.icu version of the collation root data",
     ]
-    FILENAME_PATTERNS = [r"genrb", r"uconv"]
+    FILENAME_PATTERNS = [
+        # check if it's the library instead of the command line utils
+        r"international_components_for_unicode.o",
+        # command line utils
+        r"genrb",
+        r"uconv",
+    ]
     VERSION_PATTERNS = [
         r"icu(?:-|/)([0-9]+\.[0-9]+(\.[0-9]+)?)",
         r"ICU ([0-9]+\.[0-9]+(\.[0-9]+)?)",

cve_bin_tool/checkers/libcurl.py

@@ -15,6 +15,8 @@
 
 class LibcurlChecker(Checker):
     CONTAINS_PATTERNS: list[str] = []
-    FILENAME_PATTERNS: list[str] = []
+    FILENAME_PATTERNS: list[str] = [
+        r"libcurl\.so(\.\d+)?",
+    ]
     VERSION_PATTERNS = [r"libcurl[ -/]([678]+\.[0-9]+\.[0-9]+)"]
     VENDOR_PRODUCT = [("haxx", "libcurl")]

cve_bin_tool/checkers/sqlite.py

@@ -6,7 +6,7 @@
 CVE checker for sqlite
 
 References:
-SQLLite version/sha1 data from https://www.sqlite.org/changes.html
+SQLite version/sha1 data from https://www.sqlite.org/changes.html
 
 CVE list: https://www.cvedetails.com/vulnerability-list/vendor_id-9237/product_id-16355/Sqlite-Sqlite.html
 
@@ -81,14 +81,14 @@ def guess_contains(self, lines):
         # If that fails, find a signature that might indicate presence of sqlite
         return super().guess_contains(lines)
 
-    def get_version(self, lines, filename):
+    def get_versions(self, lines, filename):
         """returns version information for sqlite as found in a given file.
 
         The most correct way to do this is to search for the sha1 sums per release.
         Fedora rpms have a simpler SQLite version string.
         """
 
-        version_info = super().get_version(lines, filename)
+        version_info = super().get_versions(lines, filename)
 
         for mapping in self.VERSION_MAP:
             # Truncate last four characters as "If the source code has been edited
@@ -97,6 +97,6 @@ def get_version(self, lines, filename):
             # https://www.sqlite.org/c3ref/c_source_id.html
             if mapping[1][:-4] in lines:
                 # overwrite version with the version found by sha mapping
-                version_info["version"] = mapping[0]
+                version_info.versions.append(mapping[0])
 
         return version_info

cve_bin_tool/util.py

@@ -194,6 +194,30 @@ class ScanInfo(NamedTuple):
     file_path: str
 
 
+class VersionMatchInfo:
+    """
+    Class to hold version match information
+    attributes:
+        matched_filename: bool
+        matched_contains: bool
+        versions: list[str]
+    """
+
+    matched_filename: bool
+    matched_contains: bool
+    versions: list[str]
+
+    def __init__(
+        self,
+        matched_filename: bool = False,
+        matched_contains: bool = False,
+        versions: list[str] = [],
+    ):
+        self.matched_filename = matched_filename
+        self.matched_contains = matched_contains
+        self.versions = versions or []
+
+
 class VersionInfo(NamedTuple):
     """
     Class to hold version information of a product
@@ -241,23 +265,32 @@ def __missing__(self, key: str) -> list[CVE] | set[str]:
 
 def regex_find(
     lines: str, version_patterns: list[Pattern[str]], ignore: list[Pattern[str]]
-) -> str:
-    """Search a set of lines to find a match for the given regex"""
-    new_guess = ""
+) -> list[str]:
+    """Search a set of lines to find all matches for the given regex"""
+    versions = list()
 
     for pattern in version_patterns:
-        match = pattern.search(lines)
-        if match:
-            new_guess = match.group(1).strip()
-            for i in ignore:
-                if str(i) in str(new_guess) or str(new_guess) in str(i):
-                    new_guess = ""
-            break
-    if new_guess != "":
-        new_guess = new_guess.replace("_", ".")
-        return new_guess.replace("-", ".")
-    else:
-        return "UNKNOWN"
+        version_matches = pattern.finditer(lines)
+        for match in version_matches:
+            # before collecting a potential version number, ensure the version string isn't on the ignore list
+            if not check_ignored(match.string, ignore):
+                version = match.group(1).strip()
+                version = version.replace("_", ".").replace("-", ".")
+                versions.append(version)
+
+    # if we searched and found no matches, just return a one-element list containing "UNKNOWN"
+    if not versions:
+        versions.append("UNKNOWN")
+
+    return versions
+
+
+def check_ignored(possible_version_string: str, ignore: list[Pattern[str]]) -> bool:
+    """Check if a possible version should be ignored based on the ignore patterns."""
+    for pattern in ignore:
+        if pattern.search(possible_version_string):
+            return True
+    return False
 
 
 def inpath(binary: str) -> bool:

cve_bin_tool/version_scanner.py

@@ -136,7 +136,7 @@ def print_language_checkers(self) -> None:
         self.logger.info(f'Language Checkers: {", ".join(self.language_checkers)}')
 
     def number_of_language_checkers(self) -> int:
-        """return the number of langauge checkers avaialble"""
+        """return the number of language checkers available"""
         return len(self.language_checkers)
 
     def is_executable(self, filename: str) -> tuple[bool, str | None]:
@@ -254,31 +254,19 @@ def run_checkers(self, filename: str, lines: str) -> Iterator[ScanInfo]:
         # tko
         for dummy_checker_name, checker in self.checkers.items():
             checker = checker()
-            result = checker.get_version(lines, filename)
-            # do some magic so we can iterate over all results, even the ones that just return 1 hit
-            if "is_or_contains" in result:
-                results = [dict()]
-                results[0] = result
-            else:
-                results = result
-
-            for result in results:
-                if "is_or_contains" in result:
-                    version = "UNKNOWN"
-                    if "version" in result and result["version"] != "UNKNOWN":
-                        version = result["version"]
-                    elif result["version"] == "UNKNOWN":
+            version_results = checker.get_versions(lines, filename)
+
+            if version_results.matched_filename or version_results.matched_contains:
+                for version in version_results.versions:
+                    if version == "UNKNOWN":
                         file_path = "".join(self.file_stack)
                         self.logger.debug(
                             f"{dummy_checker_name} was detected with version UNKNOWN in file {file_path}"
                         )
                     else:
-                        self.logger.error(f"No version info for {dummy_checker_name}")
-
-                    if version != "UNKNOWN":
                         file_path = "".join(self.file_stack)
                         self.logger.debug(
-                            f'{file_path} {result["is_or_contains"]} {dummy_checker_name} {version}'
+                            f"{file_path} matched {dummy_checker_name} {version} ({version_results.matched_filename=}, {version_results.matched_contains=})"
                         )
                         for vendor, product in checker.VENDOR_PRODUCT:
                             yield ScanInfo(
@@ -292,7 +280,7 @@ def run_checkers(self, filename: str, lines: str) -> Iterator[ScanInfo]:
     def clean_file_path(filepath: str) -> str:
         """Returns a cleaner filepath by removing temp path from filepath"""
 
-        # we'll recieve a filepath similar to
+        # we'll receive a filepath similar to
         # /temp/anything/extractable_filename.extracted/folders/inside/file
         # We'll return /folders/inside/file to be scanned
 

test/README.md

@@ -160,7 +160,7 @@ To test the filename mappings, rather than making a bunch of empty files, we're
     )
 ```
 
-The function test_filename_is will then load the checker you have specified (and fail spectacularly if you specify a checker that does not exist), try to run get_version() with an empty file content and the filename you specified, then check that it "is" something (as opposed to "contains") and that the modulename that get_version returns is in fact the expected_result you specified.
+The function test_filename_is will then load the checker you have specified (and fail spectacularly if you specify a checker that does not exist), try to run get_versions() with an empty file content and the filename you specified, then check that it "is" something (as opposed to "contains") and that the modulename that get_version returns is in fact the expected_result you specified.
 
 For ease of maintenance, please keep the parametrize list in alphabetical order when you add a new tests.