-
Notifications
You must be signed in to change notification settings - Fork 613
fix: skip SBOM components with UNKNOWN version instead of raising exception #5646
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -17,7 +17,7 @@ | |||||||||||||||||||||||||||||||||||||||
| from cve_bin_tool.log import LOGGER | ||||||||||||||||||||||||||||||||||||||||
| from cve_bin_tool.theme import cve_theme | ||||||||||||||||||||||||||||||||||||||||
| from cve_bin_tool.util import CVE, CVEData, ProductInfo, Remarks, VersionInfo | ||||||||||||||||||||||||||||||||||||||||
| from cve_bin_tool.version_compare import Version | ||||||||||||||||||||||||||||||||||||||||
| from cve_bin_tool.version_compare import UnknownVersion, Version | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| class CVEScanner: | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -149,7 +149,14 @@ def get_cves(self, product_info: ProductInfo, triage_data: TriageData): | |||||||||||||||||||||||||||||||||||||||
| vendor = product_info.vendor.replace("*", "") | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| # Use our Version class to do version compares | ||||||||||||||||||||||||||||||||||||||||
| parsed_version = Version(product_info.version) | ||||||||||||||||||||||||||||||||||||||||
| # Skip components with unknown/unparseable versions instead of raising an exception | ||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||
| parsed_version = Version(product_info.version) | ||||||||||||||||||||||||||||||||||||||||
| except UnknownVersion: | ||||||||||||||||||||||||||||||||||||||||
| self.logger.debug( | ||||||||||||||||||||||||||||||||||||||||
| f"Skipping {product_info.product} due to unknown version: {product_info.version!r}" | ||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
| ) | |
| ) | |
| # Even when skipping due to unknown version, record that this product was detected | |
| # so that downstream consumers (e.g., SBOM generation) can see it as "detected but skipped". | |
| paths = set(triage_data.get("paths", [])) | |
| # Update all_product_data with the observed paths, if this structure is present. | |
| if hasattr(self, "all_product_data"): | |
| product_entry = self.all_product_data.setdefault( | |
| product_info, {"paths": set()} | |
| ) | |
| product_entry.setdefault("paths", set()).update(paths) | |
| # Ensure all_cve_data has an entry with no CVEs but with the correct paths. | |
| if hasattr(self, "all_cve_data"): | |
| cve_entry = self.all_cve_data.setdefault( | |
| product_info, {"cves": [], "paths": set()} | |
| ) | |
| cve_entry.setdefault("paths", set()).update(paths) |
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,89 @@ | ||||||||||||||||
| # Copyright (C) 2021 Intel Corporation | ||||||||||||||||
| # SPDX-License-Identifier: GPL-3.0-or-later | ||||||||||||||||
|
|
||||||||||||||||
| """ | ||||||||||||||||
| Tests for CVEScanner graceful handling of UNKNOWN or empty versions. | ||||||||||||||||
|
|
||||||||||||||||
| Regression tests for https://github.com/ossf/cve-bin-tool/issues/5302 | ||||||||||||||||
| A SBOM component with version "UNKNOWN" previously raised an UnknownVersion | ||||||||||||||||
| exception that terminated the entire scan. Components with unknown versions | ||||||||||||||||
| should be silently skipped instead. | ||||||||||||||||
| """ | ||||||||||||||||
| from __future__ import annotations | ||||||||||||||||
|
|
||||||||||||||||
| from unittest.mock import MagicMock | ||||||||||||||||
|
|
||||||||||||||||
| import pytest | ||||||||||||||||
|
|
||||||||||||||||
| from cve_bin_tool.cve_scanner import CVEScanner | ||||||||||||||||
| from cve_bin_tool.util import ProductInfo | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| TRIAGE_DATA_EMPTY: dict = {"paths": {""}} | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| def make_scanner() -> CVEScanner: | ||||||||||||||||
| """Create a CVEScanner instance with a mocked cursor for unit testing.""" | ||||||||||||||||
| scanner = CVEScanner(score=0) | ||||||||||||||||
| scanner.cursor = MagicMock() | ||||||||||||||||
| scanner.cursor.fetchall.return_value = [] | ||||||||||||||||
| scanner.cursor.__iter__ = MagicMock(return_value=iter([])) | ||||||||||||||||
| return scanner | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| class TestCVEScannerUnknownVersion: | ||||||||||||||||
| """Tests that CVEScanner skips components with UNKNOWN or empty versions.""" | ||||||||||||||||
|
|
||||||||||||||||
| def test_unknown_version_does_not_raise(self): | ||||||||||||||||
| """SBOM component with version 'UNKNOWN' should be skipped, not crash. | ||||||||||||||||
|
|
||||||||||||||||
| Regression test for https://github.com/ossf/cve-bin-tool/issues/5302 | ||||||||||||||||
| """ | ||||||||||||||||
| scanner = make_scanner() | ||||||||||||||||
| product_info = ProductInfo( | ||||||||||||||||
| vendor="tools", | ||||||||||||||||
| product="tools", | ||||||||||||||||
| version="UNKNOWN", | ||||||||||||||||
| ) | ||||||||||||||||
| # Should not raise UnknownVersion exception | ||||||||||||||||
| scanner.get_cves(product_info, TRIAGE_DATA_EMPTY) | ||||||||||||||||
| # Component must be skipped - not added to cve_data | ||||||||||||||||
| assert product_info not in scanner.all_cve_data | ||||||||||||||||
|
|
||||||||||||||||
| def test_unknown_version_lowercase_does_not_raise(self): | ||||||||||||||||
| """SBOM component with version 'unknown' (lowercase) should be skipped.""" | ||||||||||||||||
| scanner = make_scanner() | ||||||||||||||||
| product_info = ProductInfo( | ||||||||||||||||
| vendor="tools", | ||||||||||||||||
| product="tools", | ||||||||||||||||
| version="unknown", | ||||||||||||||||
| ) | ||||||||||||||||
| scanner.get_cves(product_info, TRIAGE_DATA_EMPTY) | ||||||||||||||||
| assert product_info not in scanner.all_cve_data | ||||||||||||||||
|
|
||||||||||||||||
| def test_empty_version_does_not_raise(self): | ||||||||||||||||
| """SBOM component with empty version string should be skipped, not crash.""" | ||||||||||||||||
| scanner = make_scanner() | ||||||||||||||||
| product_info = ProductInfo( | ||||||||||||||||
| vendor="tools", | ||||||||||||||||
| product="tools", | ||||||||||||||||
| version="", | ||||||||||||||||
| ) | ||||||||||||||||
| scanner.get_cves(product_info, TRIAGE_DATA_EMPTY) | ||||||||||||||||
| assert product_info not in scanner.all_cve_data | ||||||||||||||||
|
|
||||||||||||||||
| def test_valid_version_still_processed(self): | ||||||||||||||||
| """Components with a valid version string should still be processed normally.""" | ||||||||||||||||
| scanner = make_scanner() | ||||||||||||||||
| product_info = ProductInfo( | ||||||||||||||||
| vendor="haxx", | ||||||||||||||||
| product="curl", | ||||||||||||||||
| version="7.34.0", | ||||||||||||||||
| ) | ||||||||||||||||
| # Should not raise - valid version proceeds normally | ||||||||||||||||
| try: | ||||||||||||||||
| scanner.get_cves(product_info, TRIAGE_DATA_EMPTY) | ||||||||||||||||
| except Exception as e: | ||||||||||||||||
| pytest.fail( | ||||||||||||||||
| f"get_cves raised unexpected exception for valid version: {e}" | ||||||||||||||||
| ) | ||||||||||||||||
|
||||||||||||||||
| try: | |
| scanner.get_cves(product_info, TRIAGE_DATA_EMPTY) | |
| except Exception as e: | |
| pytest.fail( | |
| f"get_cves raised unexpected exception for valid version: {e}" | |
| ) | |
| scanner.get_cves(product_info, TRIAGE_DATA_EMPTY) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UnknownVersioninversion_compare.parse_version()is detected before.strip(), so inputs like' UNKNOWN '(leading/trailing whitespace) won’t raise and can lead to incorrect range comparisons / false positives. Consider normalizingproduct_info.version(e.g., stripping whitespace and applying case-insensitive'unknown'handling) before constructingVersion()so the skip logic is applied consistently.