Skip to content

Commit 49ececf

Browse files
committed
fix(advisory): validate ids used in output paths
cveId comes verbatim from remotely-fetched CVE records and was interpolated into output filenames (<id>.csaf.json / <id>.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
1 parent 2e361f7 commit 49ececf

2 files changed

Lines changed: 85 additions & 0 deletions

File tree

central/gen-advisory

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import argparse
3434
import json
3535
import os
3636
import pathlib
37+
import re
3738
import sys
3839
import urllib.request
3940
import uuid
@@ -43,6 +44,23 @@ from datetime import datetime, timezone
4344
GEN_TOOL_NAME = 'wolfssl-advisory-gen'
4445
GEN_TOOL_VERSION = '0.3'
4546

47+
# CVE ids and advisory ids are interpolated into output filenames
48+
# (<id>.csaf.json / <id>.cdx.json) and into the CSAF self URL. Records are
49+
# fetched from a remote API and parsed as arbitrary JSON, so an unvalidated id
50+
# is an untrusted-input -> arbitrary-file-write vector (e.g. cveId '../x').
51+
# Constrain both to a safe grammar with no path separators before either is
52+
# used to build a path.
53+
_CVE_ID_RE = re.compile(r'^CVE-[0-9]{4}-[0-9]{4,}$')
54+
_ADVISORY_ID_RE = re.compile(r'^[A-Za-z0-9][A-Za-z0-9._-]*$')
55+
56+
57+
def _validate_path_id(value, kind, pattern):
58+
"""Reject an id that is unsafe to interpolate into an output path."""
59+
if not pattern.match(value):
60+
sys.exit(f"ERROR: refusing unsafe {kind} {value!r}: must match "
61+
f"{pattern.pattern} (no path separators)")
62+
return value
63+
4664
_SCRIPTS_DIR = pathlib.Path(__file__).resolve().parent
4765
_REPO_ROOT = _SCRIPTS_DIR.parent
4866

@@ -278,6 +296,7 @@ def parse_record(record):
278296
cve_id = meta.get('cveId') or cna.get('cveId')
279297
if not cve_id:
280298
sys.exit("ERROR: CVE record has no cveId")
299+
_validate_path_id(cve_id, 'cveId', _CVE_ID_RE)
281300

282301
description = ''
283302
for d in cna.get('descriptions', []):
@@ -878,6 +897,10 @@ def main():
878897
'instead of batch mode.')
879898
args = p.parse_args()
880899

900+
# --advisory-id is interpolated into output filenames; constrain it too.
901+
if args.advisory_id:
902+
_validate_path_id(args.advisory_id, '--advisory-id', _ADVISORY_ID_RE)
903+
881904
# ---- resolve the input records ----
882905
explicit = bool(args.cve_record or args.cve_id)
883906
if explicit:

central/test_gen_advisory.py

100644100755
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -720,5 +720,67 @@ def test_malformed_record_fails_without_writing(self):
720720
self.assertFalse(os.path.exists(csaf))
721721

722722

723+
class TestPathIdValidation(unittest.TestCase):
724+
"""cveId and --advisory-id are interpolated into output filenames; a
725+
record is fetched from a remote API and parsed as arbitrary JSON, so an
726+
unvalidated id is an arbitrary-file-write vector. Guard both."""
727+
728+
def _run(self, args):
729+
return subprocess.run([sys.executable, str(SCRIPT)] + args,
730+
capture_output=True, text=True)
731+
732+
@staticmethod
733+
def _record(cve_id):
734+
return {'cveMetadata': {'cveId': cve_id},
735+
'containers': {'cna': {
736+
'descriptions': [{'lang': 'en', 'value': 'test desc'}],
737+
'affected': [{'vendor': 'wolfSSL', 'product': 'wolfSSL',
738+
'versions': []}]}}}
739+
740+
def test_traversal_cveid_rejected_and_writes_nothing_outside(self):
741+
with tempfile.TemporaryDirectory() as d:
742+
rec = os.path.join(d, 'evil.json')
743+
with open(rec, 'w') as f:
744+
json.dump(self._record('../ESCAPED'), f)
745+
out = os.path.join(d, 'out', 'batch')
746+
os.makedirs(out)
747+
r = self._run(['--cve-record', rec, '--out-dir', out])
748+
self.assertNotEqual(r.returncode, 0, r.stdout)
749+
self.assertIn('unsafe cveId', r.stderr)
750+
# The escaped path (sibling of out/, i.e. d/out/ESCAPED.*) must
751+
# not have been written.
752+
escaped = os.path.join(d, 'out', 'ESCAPED.csaf.json')
753+
self.assertFalse(os.path.exists(escaped), escaped)
754+
755+
def test_absolute_cveid_rejected(self):
756+
with tempfile.TemporaryDirectory() as d:
757+
rec = os.path.join(d, 'abs.json')
758+
with open(rec, 'w') as f:
759+
json.dump(self._record('/etc/pwned'), f)
760+
r = self._run(['--cve-record', rec, '--out-dir', d])
761+
self.assertNotEqual(r.returncode, 0, r.stdout)
762+
self.assertIn('unsafe cveId', r.stderr)
763+
764+
def test_well_formed_cveid_accepted(self):
765+
with tempfile.TemporaryDirectory() as d:
766+
rec = os.path.join(d, 'good.json')
767+
with open(rec, 'w') as f:
768+
json.dump(self._record('CVE-2026-12345'), f)
769+
r = self._run(['--cve-record', rec, '--out-dir', d])
770+
self.assertEqual(r.returncode, 0, r.stderr)
771+
self.assertTrue(
772+
os.path.exists(os.path.join(d, 'CVE-2026-12345.csaf.json')))
773+
774+
def test_traversal_advisory_id_rejected(self):
775+
with tempfile.TemporaryDirectory() as d:
776+
rec = os.path.join(d, 'good.json')
777+
with open(rec, 'w') as f:
778+
json.dump(self._record('CVE-2026-12345'), f)
779+
r = self._run(['--cve-record', rec, '--out-dir', d,
780+
'--advisory-id', '../evil'])
781+
self.assertNotEqual(r.returncode, 0, r.stdout)
782+
self.assertIn('unsafe --advisory-id', r.stderr)
783+
784+
723785
if __name__ == '__main__':
724786
unittest.main()

0 commit comments

Comments
 (0)