Skip to content

Commit b3bccce

Browse files
authored
Merge pull request #25 from wolfSSL/fix/advisory-path-traversal
fix(advisory): validate ids used in output paths (path traversal)
2 parents a8dc2e3 + 1ec7d87 commit b3bccce

2 files changed

Lines changed: 87 additions & 0 deletions

File tree

central/gen-advisory

Lines changed: 25 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,25 @@ 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 isinstance(value, str):
60+
sys.exit(f"ERROR: refusing unsafe {kind}: expected string id, got {type(value).__name__}")
61+
if not pattern.fullmatch(value):
62+
sys.exit(f"ERROR: refusing unsafe {kind} {value!r}: must match "
63+
f"{pattern.pattern} (no path separators)")
64+
return value
65+
4666
_SCRIPTS_DIR = pathlib.Path(__file__).resolve().parent
4767
_REPO_ROOT = _SCRIPTS_DIR.parent
4868

@@ -278,6 +298,7 @@ def parse_record(record):
278298
cve_id = meta.get('cveId') or cna.get('cveId')
279299
if not cve_id:
280300
sys.exit("ERROR: CVE record has no cveId")
301+
_validate_path_id(cve_id, 'cveId', _CVE_ID_RE)
281302

282303
description = ''
283304
for d in cna.get('descriptions', []):
@@ -878,6 +899,10 @@ def main():
878899
'instead of batch mode.')
879900
args = p.parse_args()
880901

902+
# --advisory-id is interpolated into output filenames; constrain it too.
903+
if args.advisory_id:
904+
_validate_path_id(args.advisory_id, '--advisory-id', _ADVISORY_ID_RE)
905+
881906
# ---- resolve the input records ----
882907
explicit = bool(args.cve_record or args.cve_id)
883908
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)