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)