From c412e9f61507dc8ead88fe27025ed48f53e7545f Mon Sep 17 00:00:00 2001 From: Mark Atwood Date: Thu, 23 Jul 2026 15:49:00 -0700 Subject: [PATCH] fix(advisory): validate ids used in output paths cveId comes verbatim from remotely-fetched CVE records and was interpolated into output filenames (.csaf.json / .cdx.json) and the CSAF self URL without validation -- an untrusted-input -> arbitrary-file-write vector (e.g. cveId "../ESCAPED" escaped --out-dir). Constrain cveId to ^CVE-[0-9]{4}-[0-9]{4,}$ and --advisory-id to a path-safe grammar before either is used to build a path. Fixes #1 Adds a TestPathIdValidation regression class. Fixes #1 --- central/gen-advisory | 23 +++++++++++++ central/test_gen_advisory.py | 62 ++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+) mode change 100644 => 100755 central/test_gen_advisory.py diff --git a/central/gen-advisory b/central/gen-advisory index 90a71b1..b4c7522 100755 --- a/central/gen-advisory +++ b/central/gen-advisory @@ -34,6 +34,7 @@ import argparse import json import os import pathlib +import re import sys import urllib.request import uuid @@ -43,6 +44,23 @@ from datetime import datetime, timezone GEN_TOOL_NAME = 'wolfssl-advisory-gen' GEN_TOOL_VERSION = '0.3' +# CVE ids and advisory ids are interpolated into output filenames +# (.csaf.json / .cdx.json) and into the CSAF self URL. Records are +# fetched from a remote API and parsed as arbitrary JSON, so an unvalidated id +# is an untrusted-input -> arbitrary-file-write vector (e.g. cveId '../x'). +# Constrain both to a safe grammar with no path separators before either is +# used to build a path. +_CVE_ID_RE = re.compile(r'^CVE-[0-9]{4}-[0-9]{4,}$') +_ADVISORY_ID_RE = re.compile(r'^[A-Za-z0-9][A-Za-z0-9._-]*$') + + +def _validate_path_id(value, kind, pattern): + """Reject an id that is unsafe to interpolate into an output path.""" + if not pattern.match(value): + sys.exit(f"ERROR: refusing unsafe {kind} {value!r}: must match " + f"{pattern.pattern} (no path separators)") + return value + _SCRIPTS_DIR = pathlib.Path(__file__).resolve().parent _REPO_ROOT = _SCRIPTS_DIR.parent @@ -258,6 +276,7 @@ def parse_record(record): cve_id = meta.get('cveId') or cna.get('cveId') if not cve_id: sys.exit("ERROR: CVE record has no cveId") + _validate_path_id(cve_id, 'cveId', _CVE_ID_RE) description = '' for d in cna.get('descriptions', []): @@ -858,6 +877,10 @@ def main(): 'instead of batch mode.') args = p.parse_args() + # --advisory-id is interpolated into output filenames; constrain it too. + if args.advisory_id: + _validate_path_id(args.advisory_id, '--advisory-id', _ADVISORY_ID_RE) + # ---- resolve the input records ---- explicit = bool(args.cve_record or args.cve_id) if explicit: diff --git a/central/test_gen_advisory.py b/central/test_gen_advisory.py old mode 100644 new mode 100755 index 4f329a0..98ee7f3 --- a/central/test_gen_advisory.py +++ b/central/test_gen_advisory.py @@ -716,5 +716,67 @@ def test_malformed_record_fails_without_writing(self): self.assertFalse(os.path.exists(csaf)) +class TestPathIdValidation(unittest.TestCase): + """cveId and --advisory-id are interpolated into output filenames; a + record is fetched from a remote API and parsed as arbitrary JSON, so an + unvalidated id is an arbitrary-file-write vector. Guard both.""" + + def _run(self, args): + return subprocess.run([sys.executable, str(SCRIPT)] + args, + capture_output=True, text=True) + + @staticmethod + def _record(cve_id): + return {'cveMetadata': {'cveId': cve_id}, + 'containers': {'cna': { + 'descriptions': [{'lang': 'en', 'value': 'test desc'}], + 'affected': [{'vendor': 'wolfSSL', 'product': 'wolfSSL', + 'versions': []}]}}} + + def test_traversal_cveid_rejected_and_writes_nothing_outside(self): + with tempfile.TemporaryDirectory() as d: + rec = os.path.join(d, 'evil.json') + with open(rec, 'w') as f: + json.dump(self._record('../ESCAPED'), f) + out = os.path.join(d, 'out', 'batch') + os.makedirs(out) + r = self._run(['--cve-record', rec, '--out-dir', out]) + self.assertNotEqual(r.returncode, 0, r.stdout) + self.assertIn('unsafe cveId', r.stderr) + # The escaped path (sibling of out/, i.e. d/out/ESCAPED.*) must + # not have been written. + escaped = os.path.join(d, 'out', 'ESCAPED.csaf.json') + self.assertFalse(os.path.exists(escaped), escaped) + + def test_absolute_cveid_rejected(self): + with tempfile.TemporaryDirectory() as d: + rec = os.path.join(d, 'abs.json') + with open(rec, 'w') as f: + json.dump(self._record('/etc/pwned'), f) + r = self._run(['--cve-record', rec, '--out-dir', d]) + self.assertNotEqual(r.returncode, 0, r.stdout) + self.assertIn('unsafe cveId', r.stderr) + + def test_well_formed_cveid_accepted(self): + with tempfile.TemporaryDirectory() as d: + rec = os.path.join(d, 'good.json') + with open(rec, 'w') as f: + json.dump(self._record('CVE-2026-12345'), f) + r = self._run(['--cve-record', rec, '--out-dir', d]) + self.assertEqual(r.returncode, 0, r.stderr) + self.assertTrue( + os.path.exists(os.path.join(d, 'CVE-2026-12345.csaf.json'))) + + def test_traversal_advisory_id_rejected(self): + with tempfile.TemporaryDirectory() as d: + rec = os.path.join(d, 'good.json') + with open(rec, 'w') as f: + json.dump(self._record('CVE-2026-12345'), f) + r = self._run(['--cve-record', rec, '--out-dir', d, + '--advisory-id', '../evil']) + self.assertNotEqual(r.returncode, 0, r.stdout) + self.assertIn('unsafe --advisory-id', r.stderr) + + if __name__ == '__main__': unittest.main()