Skip to content

Commit 2f5e895

Browse files
author
Ubuntu
committed
Fix CVE version-range matching: numeric comparison + Excluding bounds
query_cves() (the primary/default correlation path once the SQLite CVE snapshot is installed) compared version_start/version_end with plain SQL >=/<=, which is a lexicographic string comparison, not numeric. "2.4.9" sorts after "2.4.10", so ranges spanning a digit-width boundary silently produced both false negatives (real vulnerabilities missed) and false positives (patched versions flagged). Version filtering now happens in Python via cve_matcher.version_in_range, which does real semver comparison. Both the SQLite ingestion path (_extract_cpe_matches_from_node) and the JSON-fallback path (cve_matcher.extract_cve_info) also only read NVD's versionStartIncluding/versionEndIncluding fields, ignoring versionStartExcluding/versionEndExcluding entirely. Since NVD commonly expresses "fixed in version X" as an Excluding bound, this silently dropped the upper bound for many CVEs (every later version stayed "vulnerable" forever) and mistreated Excluding boundaries as inclusive elsewhere. Both extraction paths now capture and honor exclusivity. version_in_range's fallback for versions packaging.version can't parse (e.g. Debian/Ubuntu-suffixed versions like "2.4.41-1ubuntu1", which are extremely common in real banners) used to unconditionally `return True` for any bounded range, which is the opposite of what this module exists to prevent. It now recovers the leading numeric version when possible and otherwise declines to claim a match rather than flooding results. cve_matcher.py had zero test coverage before this change.
1 parent e6b5846 commit 2f5e895

4 files changed

Lines changed: 349 additions & 59 deletions

File tree

bitprobe/scanner/cve_db_manager.py

Lines changed: 55 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
from contextlib import closing
1414
from datetime import datetime, timedelta, timezone
1515
from typing import List, Dict, Optional, Any
16-
from packaging import version
1716
from pathlib import Path
1817
from scanner.paths import CVE_DB_PATH, CVE_META_PATH, migrate_legacy_cve_database
1918
from scanner.update_lock import bitsentry_update_lock
@@ -1067,13 +1066,36 @@ def _extract_cpe_matches_from_node(node: Dict) -> List[Dict]:
10671066
product = parts[4] if len(parts) > 4 else ''
10681067
version_str = parts[5] if len(parts) > 5 else '*'
10691068

1069+
# Including and Excluding are mutually exclusive per NVD's schema;
1070+
# track which applied so an Excluding bound (that version is
1071+
# already patched) isn't treated as inclusive downstream.
1072+
version_start = match.get('versionStartIncluding')
1073+
version_start_including = 1
1074+
if version_start is None:
1075+
version_start = match.get('versionStartExcluding')
1076+
if version_start is not None:
1077+
version_start_including = 0
1078+
1079+
version_end = match.get('versionEndIncluding')
1080+
version_end_including = 1
1081+
if version_end is None:
1082+
version_end = match.get('versionEndExcluding')
1083+
if version_end is not None:
1084+
version_end_including = 0
1085+
1086+
if version_start is None and version_end is None and version_str != '*':
1087+
# Exact-version CPE entry (e.g. "...:1.18.0:*:*:..."): treat as
1088+
# an inclusive single-version range.
1089+
version_start = version_str
1090+
version_end = version_str
1091+
10701092
products.append({
10711093
'vendor': vendor.lower(),
10721094
'product': product.lower(),
1073-
'version_start': match.get('versionStartIncluding', version_str if version_str != '*' else None),
1074-
'version_end': match.get('versionEndIncluding', version_str if version_str != '*' else None),
1075-
'version_start_including': 1 if match.get('versionStartIncluding') else 0,
1076-
'version_end_including': 1 if match.get('versionEndIncluding') else 0,
1095+
'version_start': version_start,
1096+
'version_end': version_end,
1097+
'version_start_including': version_start_including,
1098+
'version_end_including': version_end_including,
10771099
})
10781100

10791101
for child in node.get('children', []):
@@ -1235,6 +1257,12 @@ def query_cves(
12351257
Uses PRODUCT_ALIASES from cve_matcher to resolve detected technology names
12361258
to their known CPE product identifiers, avoiding false positives from
12371259
substring matching (e.g., 'astro' no longer matches 'astrocam').
1260+
1261+
Version filtering is done in Python (not SQL): version_start/version_end
1262+
are dotted version strings, and comparing them with SQL's ">="/"<="
1263+
does a lexicographic string comparison, not a numeric one (e.g. the
1264+
string "2.4.9" sorts after "2.4.10"), which silently produces both
1265+
false positives and false negatives. See scanner.cve_matcher.version_in_range.
12381266
12391267
Args:
12401268
product: Product name (e.g., "nginx", "wordpress")
@@ -1247,7 +1275,7 @@ def query_cves(
12471275
if not os.path.exists(CVE_DB_PATH):
12481276
raise FileNotFoundError("CVE database not found. Run 'bitprobe update-cve-db' first.")
12491277

1250-
from scanner.cve_matcher import _get_cpe_names, _get_expected_vendor
1278+
from scanner.cve_matcher import _get_cpe_names, _get_expected_vendor, version_in_range
12511279

12521280
cpe_names = _get_cpe_names(product)
12531281
if not cpe_names:
@@ -1265,7 +1293,9 @@ def query_cves(
12651293
query = f"""
12661294
SELECT DISTINCT
12671295
c.cve_id, c.description, c.severity,
1268-
c.cvss_score, c."references", c.published_date
1296+
c.cvss_score, c."references", c.published_date,
1297+
p.version_start, p.version_end,
1298+
p.version_start_including, p.version_end_including
12691299
FROM cve_entries c
12701300
JOIN cve_products p ON c.cve_id = p.cve_id
12711301
WHERE p.product IN ({placeholders})
@@ -1279,34 +1309,37 @@ def query_cves(
12791309
query += " AND p.vendor = ?"
12801310
params.append(expected_vendor)
12811311

1282-
# Version matching if provided
1283-
if version:
1284-
query += """
1285-
AND (
1286-
(p.version_start IS NULL OR ? >= p.version_start)
1287-
AND (p.version_end IS NULL OR ? <= p.version_end)
1288-
)
1289-
"""
1290-
params.extend([version, version])
1291-
12921312
query += " ORDER BY c.cvss_score DESC NULLS LAST"
12931313

12941314
cursor.execute(query, params)
12951315
rows = cursor.fetchall()
12961316

1297-
cves = []
1317+
cves = {}
12981318
for row in rows:
1299-
cve = {
1300-
'cve_id': row['cve_id'],
1319+
cve_id = row['cve_id']
1320+
if cve_id in cves:
1321+
# Already matched via a different product/version row for
1322+
# this CVE; each row is an independent vulnerable
1323+
# configuration, so one match is enough.
1324+
continue
1325+
if version and not version_in_range(
1326+
version,
1327+
row['version_start'],
1328+
row['version_end'],
1329+
min_inclusive=bool(row['version_start_including']),
1330+
max_inclusive=bool(row['version_end_including']),
1331+
):
1332+
continue
1333+
cves[cve_id] = {
1334+
'cve_id': cve_id,
13011335
'description': row['description'],
13021336
'severity': row['severity'],
13031337
'cvss_score': row['cvss_score'],
13041338
'published_date': row['published_date'],
13051339
'references': json.loads(row['references'] or '[]')
13061340
}
1307-
cves.append(cve)
13081341

1309-
return cves
1342+
return list(cves.values())
13101343

13111344
finally:
13121345
conn.close()

bitprobe/scanner/cve_matcher.py

Lines changed: 95 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -126,38 +126,78 @@ def parse_version_range(cpe: Dict) -> Tuple[Optional[str], Optional[str]]:
126126
return (version, version)
127127

128128

129-
def version_in_range(detected: str, min_ver: Optional[str], max_ver: Optional[str]) -> bool:
130-
"""Check if detected version falls within the range."""
129+
def _coerce_version(value: str):
130+
"""
131+
Parse a version string leniently.
132+
133+
Real-world banner/package versions are frequently not valid PEP 440
134+
(e.g. Debian/Ubuntu suffixes like "2.4.41-1ubuntu1" or
135+
"5.7.31-0ubuntu0.18.04.1"), which packaging.version.parse rejects
136+
outright. Fall back to the leading dotted-numeric prefix so those
137+
versions can still be compared; return None only if no numeric
138+
version can be recovered at all.
139+
"""
140+
try:
141+
return pkg_version.parse(value)
142+
except Exception:
143+
pass
144+
match = re.match(r"[0-9]+(?:\.[0-9]+)*", value)
145+
if not match:
146+
return None
147+
try:
148+
return pkg_version.parse(match.group(0))
149+
except Exception:
150+
return None
151+
152+
153+
def version_in_range(
154+
detected: str,
155+
min_ver: Optional[str],
156+
max_ver: Optional[str],
157+
min_inclusive: bool = True,
158+
max_inclusive: bool = True,
159+
) -> bool:
160+
"""Check if detected version falls within [min_ver, max_ver].
161+
162+
min_inclusive/max_inclusive control whether each bound is inclusive
163+
(versionStartIncluding/versionEndIncluding in NVD terms) or exclusive
164+
(versionStartExcluding/versionEndExcluding).
165+
"""
131166
if not detected:
132167
# No version detected - can't determine vulnerability
133168
# Only match if CVE affects all versions (no version constraints)
134169
return min_ver is None and max_ver is None
135-
136-
try:
137-
detected_v = pkg_version.parse(detected)
138-
139-
if min_ver and max_ver:
140-
# Specific version or range
141-
if min_ver == max_ver:
142-
return detected_v == pkg_version.parse(min_ver)
143-
return pkg_version.parse(min_ver) <= detected_v <= pkg_version.parse(max_ver)
144-
145-
elif min_ver:
146-
return detected_v >= pkg_version.parse(min_ver)
147-
148-
elif max_ver:
149-
return detected_v <= pkg_version.parse(max_ver)
150-
151-
else:
152-
# No version constraints - any version matches
153-
return True
154-
155-
except Exception:
156-
# Fallback to string comparison
157-
if min_ver and max_ver and min_ver == max_ver:
158-
return detected == min_ver
170+
171+
if min_ver is None and max_ver is None:
159172
return True
160173

174+
detected_v = _coerce_version(detected)
175+
if detected_v is None:
176+
# Can't parse the detected version at all - don't claim a match
177+
# against a bounded range; that would flood results with false
178+
# positives for every technology we can't version-compare.
179+
return False
180+
181+
if min_ver is not None:
182+
min_v = _coerce_version(min_ver)
183+
if min_v is not None:
184+
if min_inclusive:
185+
if detected_v < min_v:
186+
return False
187+
elif detected_v <= min_v:
188+
return False
189+
190+
if max_ver is not None:
191+
max_v = _coerce_version(max_ver)
192+
if max_v is not None:
193+
if max_inclusive:
194+
if detected_v > max_v:
195+
return False
196+
elif detected_v >= max_v:
197+
return False
198+
199+
return True
200+
161201

162202
def extract_cve_info(cve_entry: Dict) -> List[Dict]:
163203
"""
@@ -180,21 +220,35 @@ def extract_cve_info(cve_entry: Dict) -> List[Dict]:
180220
if not cpe:
181221
continue
182222

183-
# Check for version range in versionEndExcluding/versionEndIncluding
184-
version_start = match.get("versionStartIncluding")
185-
version_end = match.get("versionEndExcluding") or match.get("versionEndIncluding")
186-
187-
if version_start or version_end:
188-
min_ver = version_start
189-
max_ver = version_end
190-
else:
223+
# Check for version range in versionStart/EndIncluding/Excluding.
224+
# Including and Excluding are mutually exclusive per NVD's
225+
# schema; track which one applied so callers can honor the
226+
# correct boundary (an Excluding bound means that version
227+
# itself is already patched).
228+
min_ver = match.get("versionStartIncluding")
229+
min_inclusive = True
230+
if min_ver is None:
231+
min_ver = match.get("versionStartExcluding")
232+
min_inclusive = False
233+
234+
max_ver = match.get("versionEndIncluding")
235+
max_inclusive = True
236+
if max_ver is None:
237+
max_ver = match.get("versionEndExcluding")
238+
max_inclusive = False
239+
240+
if min_ver is None and max_ver is None:
191241
min_ver, max_ver = parse_version_range(cpe)
242+
min_inclusive = True
243+
max_inclusive = True
192244

193245
products.append({
194246
"vendor": cpe.get("vendor", ""),
195247
"product": cpe.get("product", ""),
196248
"min_version": min_ver,
197249
"max_version": max_ver,
250+
"min_inclusive": min_inclusive,
251+
"max_inclusive": max_inclusive,
198252
"version": cpe.get("version"),
199253
})
200254

@@ -211,7 +265,13 @@ def match_technology_to_cve(tech_name: str, tech_version: Optional[str], cve_ent
211265
for product in affected_products:
212266
if product_names_match(tech_name, product["product"]):
213267
# Product matches, check version
214-
if version_in_range(tech_version, product["min_version"], product["max_version"]):
268+
if version_in_range(
269+
tech_version,
270+
product["min_version"],
271+
product["max_version"],
272+
min_inclusive=product.get("min_inclusive", True),
273+
max_inclusive=product.get("max_inclusive", True),
274+
):
215275
return {
216276
"matched_product": product["product"],
217277
"detected_version": tech_version,

0 commit comments

Comments
 (0)