Reject unsafe filesystem paths from the HACS manifest

custom_components/hacs/repositories/base.py

@@ -36,7 +36,7 @@
 from ..utils.github_graphql_query import GET_REPOSITORY_RELEASES
 from ..utils.json import json_loads
 from ..utils.logger import LOGGER
-from ..utils.path import is_safe
+from ..utils.path import is_safe, is_safe_relative_path
 from ..utils.queue_manager import QueueManager
 from ..utils.store import async_remove_store
 from ..utils.url import github_archive, github_release_asset
@@ -255,6 +255,16 @@ def from_dict(manifest: dict):
                 setattr(manifest_data, key, [value])
             elif key in manifest_data.__dict__:
                 setattr(manifest_data, key, value)
+
+        # These end up in filesystem paths, a hostile manifest must not
+        # be able to point them outside the repository content directory.
+        for key in ("filename", "persistent_directory"):
+            value = getattr(manifest_data, key)
+            if value is not None and not is_safe_relative_path(value):
+                LOGGER.warning("Ignoring unsafe %s value '%s' in the HACS manifest", key, value)
+                setattr(manifest_data, key, None)
+                manifest_data.manifest.pop(key, None)
+
         return manifest_data
 
     def update_data(self, data: dict) -> None:

custom_components/hacs/utils/path.py

@@ -3,7 +3,7 @@
 from __future__ import annotations
 
 from functools import lru_cache
-from pathlib import Path
+from pathlib import Path, PureWindowsPath
 from typing import TYPE_CHECKING
 
 if TYPE_CHECKING:
@@ -39,3 +39,15 @@ def is_safe(hacs: HacsBase, path: str | Path) -> bool:
         configuration.python_script_path,
         configuration.theme_path,
     )
+
+
+def is_safe_relative_path(value: str) -> bool:
+    """Check that a repository provided path is relative, without traversal."""
+    if not isinstance(value, str):
+        return False
+
+    normalized = value.replace("\\", "/")
+    if normalized.startswith("/") or PureWindowsPath(value).drive:
+        return False
+
+    return ".." not in normalized.split("/")

custom_components/hacs/utils/validate.py

@@ -11,6 +11,7 @@
 import voluptuous as vol
 
 from ..const import LOCALE
+from .path import is_safe_relative_path
 
 
 @dataclass
@@ -43,15 +44,25 @@ def _country_validator(values) -> list[str]:
     return countries
 
 
+def _relative_path_validator(value) -> str:
+    """Custom validator for repository provided paths."""
+    if not isinstance(value, str):
+        raise vol.Invalid(f"Value '{value}' is not a string.")
+    if not is_safe_relative_path(value):
+        raise vol.Invalid(f"Value '{value}' is not a safe relative path.")
+
+    return value
+
+
 HACS_MANIFEST_JSON_SCHEMA = vol.Schema(
     {
         vol.Optional("content_in_root"): bool,
         vol.Optional("country"): _country_validator,
-        vol.Optional("filename"): str,
+        vol.Optional("filename"): _relative_path_validator,
         vol.Optional("hacs"): str,
         vol.Optional("hide_default_branch"): bool,
         vol.Optional("homeassistant"): str,
-        vol.Optional("persistent_directory"): str,
+        vol.Optional("persistent_directory"): _relative_path_validator,
         vol.Optional("render_readme"): bool,
         vol.Optional("zip_release"): bool,
         vol.Required("name"): str,

tests/repositories/test_hacs_manifest.py

@@ -42,3 +42,29 @@ def test_manifest_structure():
 def test_edge_pass_none():
     with pytest.raises(HacsException):
         assert HacsManifest.from_dict(None)
+
+
+@pytest.mark.parametrize("key", ["filename", "persistent_directory"])
+def test_unsafe_paths_are_ignored(key: str, caplog: pytest.LogCaptureFixture):
+    manifest = HacsManifest.from_dict({"name": "TEST", key: "../../../evil"})
+
+    assert getattr(manifest, key) is None
+    assert key not in manifest.manifest
+    assert f"Ignoring unsafe {key} value '../../../evil' in the HACS manifest" in caplog.text
+
+
+@pytest.mark.parametrize("key", ["filename", "persistent_directory"])
+def test_safe_paths_are_kept(key: str):
+    manifest = HacsManifest.from_dict({"name": "TEST", key: "sub/dir"})
+
+    assert getattr(manifest, key) == "sub/dir"
+
+
+@pytest.mark.parametrize("value", [False, 0, 123, ["list"]])
+@pytest.mark.parametrize("key", ["filename", "persistent_directory"])
+def test_non_string_paths_are_ignored(key: str, value, caplog: pytest.LogCaptureFixture):
+    manifest = HacsManifest.from_dict({"name": "TEST", key: value})
+
+    assert getattr(manifest, key) is None
+    assert key not in manifest.manifest
+    assert f"Ignoring unsafe {key} value '{value}' in the HACS manifest" in caplog.text

tests/utils/test_path.py

@@ -1,3 +1,5 @@
+import pytest
+
 from custom_components.hacs.base import HacsBase
 from custom_components.hacs.utils import path
 
@@ -7,3 +9,28 @@ def test_is_safe(hacs: HacsBase) -> None:
     assert not path.is_safe(hacs, f"{hacs.core.config_path}/{hacs.configuration.theme_path}/")
     assert not path.is_safe(hacs, f"{hacs.core.config_path}/custom_components/")
     assert not path.is_safe(hacs, f"{hacs.core.config_path}/custom_components")
+
+
+@pytest.mark.parametrize(
+    ("value", "expected"),
+    [
+        ("example.js", True),
+        ("sub/dir/example.js", True),
+        ("userfiles", True),
+        ("..", False),
+        ("../example.js", False),
+        ("sub/../../example.js", False),
+        ("/etc/passwd", False),
+        ("\\windows\\style", False),
+        ("sub\\..\\..\\example.js", False),
+        ("C:\\windows\\style", False),
+        ("C:/windows/style", False),
+        ("C:windows\\style", False),
+        ("//server/share", False),
+        (None, False),
+        (123, False),
+        (False, False),
+    ],
+)
+def test_is_safe_relative_path(value, expected: bool) -> None:
+    assert path.is_safe_relative_path(value) is expected

tests/utils/test_validate.py

@@ -88,6 +88,20 @@ def test_hacs_manifest_json_schema():
     with pytest.raises(Invalid, match=re.escape("Value 'False' is not a string or list.")):
         hacs_json_schema({"name": "My awesome thing", "country": False})
 
+    for key in ("filename", "persistent_directory"):
+        with pytest.raises(
+            Invalid, match=re.escape("Value '../secrets' is not a safe relative path."),
+        ):
+            hacs_json_schema({"name": "My awesome thing", key: "../secrets"})
+
+        with pytest.raises(
+            Invalid, match=re.escape("Value '/etc/passwd' is not a safe relative path."),
+        ):
+            hacs_json_schema({"name": "My awesome thing", key: "/etc/passwd"})
+
+        with pytest.raises(Invalid, match=re.escape("Value 'False' is not a string.")):
+            hacs_json_schema({"name": "My awesome thing", key: False})
+
 
 def test_integration_json_schema():
     """Test integration manifest."""