Skip to content

Commit 19ebc90

Browse files
MarkAtwoodsameehj
authored andcommitted
fix(sbom): correctness fixes in gen-sbom
Three review-driven correctness fixes in scripts/gen-sbom, each shipping with its own regression tests: * parse_options_h corrupted #define values that contained a string literal with '/' characters: 're.split(r"/\\*|//", raw, maxsplit=1)[0]' truncated PACKAGE_URL="https://..." at the first // inside the URL, so the SBOM emitted 'PACKAGE_URL = "https:'. Replace the regex with a string-literal-aware state machine (_strip_define_comment) so quoted '/' characters survive. Char literals are not handled (autoconf does not emit them, pcpp normalises user_settings.h before this code sees the value). Adds four regression tests to TestParseOptionsH (1zj.22). * --lib accepted /dev/null and emitted the well-known empty- file SHA-256 (e3b0c44...b855) as the wolfSSL component checksum. Both SPDX and CDX validators accepted the result, so a misconfigured build silently shipped an SBOM whose hash matched no compiled wolfSSL artefact. Hard-fail on size==0 --lib, with an actionable error. Soft-warn on zero-byte --srcs entries because cross-compile setups legitimately include touch'd placeholders. Adds test_empty_lib_is_rejected and test_zero_byte_srcs_warn_but_do_not_fail (1zj.23). * detect_license regex 'or\\s+(any\\s+)?later' failed to match the canonical FSF GPL preamble phrase 'or (at your option) any later version' used verbatim in millions of upstream COPYING files: the parenthetical interjected between 'or' and 'any later'. Any LICENSING that copied the FSF preamble was silently mis-detected as GPL-X.0-only. Broaden to 'or\\s+(?:[^,.;\\n]*?\\s+)?(?:any\\s+)?later' -- the negative class keeps the match within a sentence so unrelated 'or' / 'later' tokens further down the file do not promote a GPLv2-only declaration. Adds GPLv2 + GPLv3 canonical-preamble regression guards. The shipping wolfSSL LICENSING still maps to GPL-3.0-only; locked down by test_real_wolfssl_licensing_is_gpl3_only (1zj.24). Also rebinds the test_zero_byte_srcs_warn_but_do_not_fail tempfile paths before the try-block so the finally clause references live filenames on the assertion-failure path instead of leaking renamed tempfiles in /tmp (e8o.2). Closes wolfssl-1zj.22, wolfssl-1zj.23, wolfssl-1zj.24, wolfssl-e8o.2
1 parent 1644688 commit 19ebc90

2 files changed

Lines changed: 224 additions & 11 deletions

File tree

scripts/gen-sbom

Lines changed: 83 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,16 @@ def detect_license(license_file):
192192

193193
version = m.group(1)
194194
excerpt = text[m.end():m.end() + 100]
195-
if re.search(r'or\s+(any\s+)?later', excerpt, re.IGNORECASE):
195+
# Match upgrade-permission wording in the 100-byte excerpt that
196+
# follows the version mention. Three FSF-derived shapes:
197+
# * canonical preamble: "or (at your option) any later version"
198+
# * preamble variant: "or (at the licensee's option) any later"
199+
# * compact form: "or later" / "or any later"
200+
# The optional `[^,.;\n]*?\s+` group consumes parenthesised
201+
# asides without crossing sentence boundaries so unrelated
202+
# "or" / "later" mentions in surrounding prose do not match.
203+
if re.search(r'or\s+(?:[^,.;\n]*?\s+)?(?:any\s+)?later',
204+
excerpt, re.IGNORECASE):
196205
return f'GPL-{version}.0-or-later'
197206
return f'GPL-{version}.0-only'
198207

@@ -338,6 +347,45 @@ def _is_noise_macro(name):
338347
return False
339348

340349

350+
def _strip_define_comment(raw):
351+
"""Strip trailing C/C++ comment from a #define value while preserving
352+
`/`-bearing characters that appear inside a double-quoted string.
353+
354+
Earlier versions used `re.split(r'/\\*|//', raw, maxsplit=1)[0]`, which
355+
is unaware of string literals. That regex corrupts autoconf-generated
356+
defines such as
357+
358+
#define PACKAGE_URL "https://www.wolfssl.com"
359+
#define PACKAGE_BUGREPORT "https://github.com/wolfssl/wolfssl/issues"
360+
361+
by truncating at the first `//` inside the URL — both end up as
362+
`"https:` in the SBOM build properties, falsely showing PACKAGE_URL
363+
drifting between releases when nothing actually changed.
364+
365+
Char literals are not handled: autoconf-generated options.h does not
366+
emit them, and pcpp normalises customer user_settings.h before this
367+
helper sees the value, so the only realistic source of `/` in a
368+
#define value is a quoted string."""
369+
in_str = False
370+
i = 0
371+
n = len(raw)
372+
while i < n:
373+
c = raw[i]
374+
if in_str:
375+
if c == '\\' and i + 1 < n:
376+
i += 2
377+
continue
378+
if c == '"':
379+
in_str = False
380+
else:
381+
if c == '"':
382+
in_str = True
383+
elif c == '/' and i + 1 < n and raw[i + 1] in '/*':
384+
return raw[:i]
385+
i += 1
386+
return raw
387+
388+
341389
def parse_options_h(path):
342390
"""Parse a flat `#define` header and return a sorted deduplicated
343391
list of (name, value) pairs for every wolfSSL-relevant macro.
@@ -354,7 +402,9 @@ def parse_options_h(path):
354402
355403
Trailing C/C++ comments on a #define line (`#define HAVE_FOO 42 /* x */`
356404
or `// y`) are stripped; otherwise they would land verbatim in the
357-
SBOM build properties."""
405+
SBOM build properties. String literals are preserved intact so that
406+
URLs in PACKAGE_URL / PACKAGE_BUGREPORT are not truncated at the
407+
first `//` (see _strip_define_comment)."""
358408
try:
359409
with open(path) as f:
360410
text = f.read()
@@ -368,7 +418,7 @@ def parse_options_h(path):
368418
if _is_noise_macro(name):
369419
continue
370420
raw = (m.group(2) or '')
371-
raw = re.split(r'/\*|//', raw, maxsplit=1)[0]
421+
raw = _strip_define_comment(raw)
372422
defines[name] = raw.strip()
373423
return sorted(defines.items())
374424

@@ -898,10 +948,40 @@ def main():
898948
)
899949

900950
if args.lib:
951+
# Refuse the empty-file SHA-256 as a component checksum. A
952+
# build that points --lib at /dev/null, a stub touch(1)'d
953+
# placeholder, or an empty .a that failed to ar-create would
954+
# otherwise emit a valid-looking SBOM whose hash matches no
955+
# compiled wolfSSL artefact ever shipped. The SBOM passes
956+
# both spec validators -- nothing else catches it.
957+
try:
958+
lib_size = os.path.getsize(args.lib)
959+
except OSError as e:
960+
sys.exit(f"ERROR: cannot stat --lib {args.lib!r}: {e}")
961+
if lib_size == 0:
962+
sys.exit(
963+
f"ERROR: --lib {args.lib!r} is empty (0 bytes); refusing "
964+
"to emit an SBOM with the empty-file SHA-256 as the "
965+
"component checksum. Verify your build produced a "
966+
"real library artefact.")
901967
lib_hash = sha256_file(args.lib)
902968
hash_kind = 'library-binary'
903969
srcs_basenames = None
904970
else:
971+
# --srcs is the embedded entry point. Zero-byte files in the
972+
# set are uncommon but not necessarily wrong (a cross-compile
973+
# toolchain may stub a per-target source with touch); warn
974+
# rather than fail so the customer can decide whether the
975+
# gitoid for an empty blob is what they want recorded.
976+
zero_byte_srcs = [
977+
p for p in args.srcs if os.path.isfile(p) and os.path.getsize(p) == 0
978+
]
979+
if zero_byte_srcs:
980+
print(
981+
"WARNING: zero-byte source files in --srcs (gitoid will "
982+
"be the well-known empty-blob hash for these): "
983+
+ ', '.join(zero_byte_srcs),
984+
file=sys.stderr)
905985
lib_hash = srcs_merkle_hash(args.srcs)
906986
hash_kind = 'source-merkle-omnibor'
907987
srcs_basenames = sorted({os.path.basename(p) for p in args.srcs})

scripts/test_gen_sbom.py

Lines changed: 141 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -306,19 +306,40 @@ def test_gplv2_only(self):
306306
def test_gplv2_or_later_any_form(self):
307307
# 'or any later' immediately after the version mention.
308308
# Oracle: SPDX 'GPL-2.0-or-later'.
309-
# NOTE: the canonical FSF preamble phrase 'or (at your option)
310-
# any later version' is NOT matched by the current regex
311-
# (the parenthetical interjection breaks the pattern). That
312-
# is a separate limitation tracked in its own beads issue;
313-
# this test deliberately uses a pattern the existing regex
314-
# handles, so it serves as a regex regression guard rather
315-
# than a redesign request.
316309
self.assertEqual(
317310
self._detect(
318311
'Licensed under the GNU General Public License version 2, '
319312
'or any later version.\n'),
320313
'GPL-2.0-or-later')
321314

315+
def test_gplv2_or_later_canonical_fsf_preamble(self):
316+
# The canonical FSF GPL preamble phrase, used verbatim in
317+
# millions of upstream COPYING files:
318+
#
319+
# 'either version N of the License, or (at your option)
320+
# any later version.'
321+
#
322+
# An earlier regex (`or\s+(any\s+)?later`) failed to match
323+
# this because the parenthetical '(at your option)'
324+
# interjects between 'or' and 'any later', so wolfssl-1zj.24
325+
# silently mis-detected the preamble as GPLv2-only. Oracle:
326+
# SPDX 'GPL-2.0-or-later'.
327+
self.assertEqual(
328+
self._detect(
329+
'This program is free software: you can redistribute it '
330+
'and/or modify it under the terms of the GNU General '
331+
'Public License version 2, or (at your option) any '
332+
'later version.\n'),
333+
'GPL-2.0-or-later')
334+
335+
def test_gplv3_or_later_canonical_fsf_preamble(self):
336+
# Same regression guard for GPLv3.
337+
self.assertEqual(
338+
self._detect(
339+
'Licensed under the GNU General Public License version 3, '
340+
'or (at your option) any later version.\n'),
341+
'GPL-3.0-or-later')
342+
322343
def test_gplv2_or_later_short_form(self):
323344
# 'or later' (without 'any') also matches the regex; this
324345
# variant appears in some upstream COPYING files. Oracle:
@@ -488,6 +509,48 @@ def test_strips_comment_from_valueless_define(self):
488509
pairs = dict(self._parse("#define HAVE_BAR /* set elsewhere */\n"))
489510
self.assertEqual(pairs['HAVE_BAR'], '')
490511

512+
def test_preserves_url_in_string_literal(self):
513+
# Regression guard: an earlier comment-stripper used
514+
# `re.split(r'/\*|//', raw, maxsplit=1)[0]`, which truncated
515+
# autoconf-generated PACKAGE_URL / PACKAGE_BUGREPORT defines
516+
# at the first `//` inside the URL. Both ended up as
517+
# `"https:` in the SBOM build properties, falsely showing
518+
# PACKAGE_URL drifting between releases when nothing changed.
519+
pairs = dict(self._parse(
520+
'#define PACKAGE_URL "https://www.wolfssl.com"\n'
521+
'#define PACKAGE_BUGREPORT '
522+
'"https://github.com/wolfssl/wolfssl/issues"\n'
523+
))
524+
self.assertEqual(pairs['PACKAGE_URL'],
525+
'"https://www.wolfssl.com"')
526+
self.assertEqual(pairs['PACKAGE_BUGREPORT'],
527+
'"https://github.com/wolfssl/wolfssl/issues"')
528+
529+
def test_strips_comment_after_string_literal(self):
530+
# Companion to test_preserves_url_in_string_literal: confirm
531+
# the stripper still works when a comment legitimately follows
532+
# a string literal. A regression that disabled stripping
533+
# entirely (the simplest "fix" for the URL bug) would let
534+
# comment text leak into the SBOM.
535+
pairs = dict(self._parse(
536+
'#define PACKAGE_URL "https://www.wolfssl.com" /* upstream */\n'
537+
))
538+
self.assertEqual(pairs['PACKAGE_URL'],
539+
'"https://www.wolfssl.com"')
540+
541+
def test_preserves_block_comment_inside_string_literal(self):
542+
# `/*` inside a string literal must not start a comment.
543+
pairs = dict(self._parse('#define WEIRD "a/*b*/c"\n'))
544+
self.assertEqual(pairs['WEIRD'], '"a/*b*/c"')
545+
546+
def test_handles_escaped_quote_in_string_literal(self):
547+
# An escaped `\"` inside a string literal must not be mistaken
548+
# for the closing quote; otherwise a comment-marker that
549+
# follows would be incorrectly treated as outside the string.
550+
pairs = dict(self._parse(
551+
'#define EMBEDDED_QUOTE "a\\"b//c" /* tail */\n'))
552+
self.assertEqual(pairs['EMBEDDED_QUOTE'], '"a\\"b//c"')
553+
491554
def test_dedup_keeps_last_assignment(self):
492555
# Last assignment wins (matches C preprocessor semantics for
493556
# duplicate #defines after redefinition).
@@ -1282,11 +1345,18 @@ def test_licenseref_with_license_text_is_accepted(self):
12821345
delete=False) as f:
12831346
f.write('Plain-text wolfSSL commercial licence text.\n')
12841347
license_text_path = f.name
1348+
# --lib must be non-empty (gen-sbom refuses /dev/null as a
1349+
# component checksum); use a tiny stand-in file so we exercise
1350+
# the LicenseRef gate without tripping the empty-lib gate.
1351+
with tempfile.NamedTemporaryFile('wb', suffix='.so',
1352+
delete=False) as f:
1353+
f.write(b'\x7fELF stub')
1354+
lib_path = f.name
12851355
try:
12861356
result = self._run(
12871357
*self.BASE,
12881358
'--options-h', '/dev/null',
1289-
'--lib', '/dev/null',
1359+
'--lib', lib_path,
12901360
'--license-override', 'LicenseRef-wolfSSL-Commercial',
12911361
'--license-text', license_text_path)
12921362
self.assertEqual(
@@ -1295,6 +1365,69 @@ def test_licenseref_with_license_text_is_accepted(self):
12951365
f'pair: stderr={result.stderr!r}')
12961366
finally:
12971367
os.unlink(license_text_path)
1368+
os.unlink(lib_path)
1369+
1370+
def test_empty_lib_is_rejected(self):
1371+
# The --lib argument is the wolfSSL component checksum source.
1372+
# An empty file produces the well-known empty-file SHA-256
1373+
# (e3b0c44...b855), which is a valid-looking hash that
1374+
# matches no real wolfSSL build artefact ever shipped. Both
1375+
# SPDX and CDX validators accept it; nothing else catches
1376+
# the lie. gen-sbom must refuse zero-byte --lib.
1377+
result = self._run(
1378+
*self.BASE,
1379+
'--options-h', '/dev/null',
1380+
'--lib', '/dev/null')
1381+
self.assertNotEqual(result.returncode, 0,
1382+
'gen-sbom accepted an empty --lib file; would '
1383+
'have shipped an SBOM with the empty-file '
1384+
'SHA-256 as the wolfSSL component checksum')
1385+
self.assertIn('empty', result.stderr.lower())
1386+
self.assertIn('--lib', result.stderr)
1387+
1388+
def test_zero_byte_srcs_warn_but_do_not_fail(self):
1389+
# Companion: --srcs may legitimately include zero-byte
1390+
# placeholders in cross-compile setups (a target file the
1391+
# build system creates with touch but doesn't compile yet),
1392+
# so gen-sbom emits a WARNING rather than failing. This
1393+
# gives the embedded customer a chance to see they have a
1394+
# stub file in the source set without breaking their build.
1395+
with tempfile.NamedTemporaryFile('wb', suffix='.c',
1396+
delete=False) as f:
1397+
f.write(b'/* real source */\n')
1398+
real_src = f.name
1399+
with tempfile.NamedTemporaryFile('wb', suffix='.c',
1400+
delete=False) as f:
1401+
empty_src = f.name
1402+
# Rename so the basenames are distinct (srcs_merkle_hash
1403+
# rejects duplicate basenames; see TestSrcsMerkleHash).
1404+
# Rename and rebind BEFORE the try-block so the finally
1405+
# clause always references the live filenames even when an
1406+
# assertion fails.
1407+
real_renamed = real_src + '.real.c'
1408+
empty_renamed = empty_src + '.empty.c'
1409+
os.rename(real_src, real_renamed)
1410+
os.rename(empty_src, empty_renamed)
1411+
real_src = real_renamed
1412+
empty_src = empty_renamed
1413+
try:
1414+
result = self._run(
1415+
*self.BASE,
1416+
'--user-settings', '/dev/null',
1417+
'--srcs', real_src, empty_src)
1418+
# The standalone path with /dev/null user-settings should
1419+
# complete; the only thing we care about here is that an
1420+
# empty source did not abort the run.
1421+
self.assertEqual(
1422+
result.returncode, 0,
1423+
f'gen-sbom failed with zero-byte source: stderr={result.stderr!r}')
1424+
self.assertIn('zero-byte source', result.stderr)
1425+
finally:
1426+
for p in (real_src, empty_src):
1427+
try:
1428+
os.unlink(p)
1429+
except FileNotFoundError:
1430+
pass
12981431

12991432
def test_user_settings_path_in_help(self):
13001433
# Discoverability regression guard - if the standalone entry

0 commit comments

Comments
 (0)