From b79f9f2b0a3716151a9cabd47aa0ce2a89854c48 Mon Sep 17 00:00:00 2001 From: Federico Capoano Date: Tue, 21 Jul 2026 13:06:06 -0300 Subject: [PATCH] [fix] Validated extra file paths #400 Extra file paths were written into generated archives and later applied by device agents. The backend now rejects malformed paths during validation so bogus archive entries do not reach downstream tools. Closes #400 [backport 1.2] (cherry picked from commit 35bab79f265e2aa36f03b7fcf3965d6d573d853b) --- docs/source/backends/openwrt.rst | 3 ++ netjsonconfig/backends/base/backend.py | 61 ++++++++++++++++++++++++++ tests/test_base.py | 47 ++++++++++++++++++++ 3 files changed, 111 insertions(+) diff --git a/docs/source/backends/openwrt.rst b/docs/source/backends/openwrt.rst index 2ba56c85a..656ecf0a5 100644 --- a/docs/source/backends/openwrt.rst +++ b/docs/source/backends/openwrt.rst @@ -2846,6 +2846,9 @@ key name type required function The ``files`` key of the *configuration dictionary* is a custom NetJSON extension not present in the original NetJSON RFC. +File paths may contain only letters, numbers, dots, underscores, dashes +and slashes. Empty path parts, ``.`` and ``..`` are not allowed. + .. warning:: The files are included in the output of the ``render`` method unless diff --git a/netjsonconfig/backends/base/backend.py b/netjsonconfig/backends/base/backend.py index de80a991c..910011a55 100644 --- a/netjsonconfig/backends/base/backend.py +++ b/netjsonconfig/backends/base/backend.py @@ -16,6 +16,9 @@ format_checker = Draft4Validator.FORMAT_CHECKER _host_name_re = re.compile(r"^[A-Za-z0-9][A-Za-z0-9\.\-]{1,255}$") +# Literal path text is intentionally limited to common path characters. +# Template variables are checked separately. +_file_path_re = re.compile(r"\A/?[A-Za-z0-9._/-]+\Z") class BaseBackend(object): @@ -158,9 +161,64 @@ def validate(self): Draft4Validator(self.schema, format_checker=format_checker).validate( self.config ) + self._validate_file_paths() except JsonSchemaError as e: raise ValidationError(e) + def _validate_file_paths(self): + """ + Validates paths used by extra configuration files. + + These paths are later written to configuration archives and applied on + devices, so accepting malformed or surprising paths can lead to unsafe + or broken behavior outside netjsonconfig. + """ + for index, file_item in enumerate(self.config.get("files", [])): + path = file_item["path"] + # Keep one leading slash valid, but do not hide extra slashes. + # They are empty path components and should fail validation. + components = ( + path[1:].split("/") if path.startswith("/") else path.split("/") + ) + valid_path = self._validate_file_path_chars(path) + valid_components = all( + component and component not in {".", ".."} for component in components + ) + if valid_path and valid_components: + continue + raise JsonSchemaError( + 'Invalid file path "{0}". Allowed characters are letters, numbers, ' + '".", "_", "-", and "/". Empty path parts and "." or ".." ' + "components are not allowed.".format(path), + path=("files", index, "path"), + ) + + def _validate_file_path_chars(self, path): + """ + Checks the visible characters of a file path. + + Template variables are internal values, so this method accepts them + only when they are properly wrapped in ``{{`` and ``}}`` while keeping + the public path character set strict and easy to explain. + """ + index = 0 + clean_path = "" + while index < len(path): + if path.startswith("{{", index): + # Variable names are not part of the path syntax. Only the + # delimiters matter here; callers decide what names are valid. + end_index = path.find("}}", index + 2) + if end_index == -1: + return False + clean_path += "x" + index = end_index + 2 + continue + if path[index] in "{}": + return False + clean_path += path[index] + index += 1 + return bool(_file_path_re.match(clean_path)) + def render(self, files=True): """ Converts the configuration dictionary into the corresponding configuration format @@ -217,6 +275,9 @@ def generate(self): :returns: in-memory tar.gz archive, instance of ``BytesIO`` """ + # Do not validate here. Old saved configs should still be downloadable + # after stricter validation is introduced; new data should be rejected + # when validate() is called. tar_bytes = BytesIO() tar = tarfile.open(fileobj=tar_bytes, mode="w") self._generate_contents(tar) diff --git a/tests/test_base.py b/tests/test_base.py index 24e63dde8..8ff498611 100644 --- a/tests/test_base.py +++ b/tests/test_base.py @@ -4,6 +4,12 @@ from netjsonconfig.backends.base.backend import BaseBackend from netjsonconfig.backends.base.parser import BaseParser from netjsonconfig.backends.base.renderer import BaseRenderer +from netjsonconfig.exceptions import ValidationError +from netjsonconfig.schema import schema + + +class BaseBackendWithSchema(BaseBackend): + schema = schema class TestBase(unittest.TestCase): @@ -32,3 +38,44 @@ def test_parse_tar_not_implemented(self): def test_base_backend_parse_not_implemented(self): with self.assertRaises(NotImplementedError): BaseBackend(native="") + + def test_validate_file_paths(self): + b = BaseBackendWithSchema( + { + "files": [ + {"path": "/etc/config/network", "mode": "0644", "contents": ""}, + {"path": "etc/openvpn/client.conf", "mode": "0644", "contents": ""}, + {"path": "{{cert_path_abc123}}", "mode": "0644", "contents": ""}, + { + "path": "/etc/{{cert_path_abc123}}", + "mode": "0644", + "contents": "", + }, + ] + } + ) + b.validate() + + def test_validate_file_paths_rejects_invalid_paths(self): + invalid_paths = [ + "tmp/x; reboot", + "../../../etc/passwd", + "tmp/a b", + "tmp/file\n", + "tmp/$(reboot)", + "//etc/passwd", + "///etc/passwd", + "tmp//file", + "tmp/./file", + "tmp/{file}", + "tmp/{{file}", + "tmp/file}}", + ] + for path in invalid_paths: + with self.subTest(path=path): + b = BaseBackendWithSchema( + {"files": [{"path": path, "mode": "0644", "contents": ""}]} + ) + with self.assertRaises(ValidationError) as context: + b.validate() + self.assertIn("Invalid file path", context.exception.message)