Add a check on the catchall path

CHANGELOG.md

@@ -35,6 +35,10 @@ The types of changes are:
 
 * Home screen header scaling and responsiveness issues [#2200](https://github.com/ethyca/fides/pull/2277)
 
+### Security
+
+* Add a check to the catchall path to prevent returning paths outside of the UI directory [#2330](https://github.com/ethyca/fides/pull/2330)
+
 ## [2.5.0](https://github.com/ethyca/fides/compare/2.4.0...2.5.0)
 
 ### Docs

src/fides/api/ctl/ui.py

@@ -8,6 +8,7 @@
 
 from fastapi import Response
 from fastapi.responses import FileResponse
+from loguru import logger
 
 FIDES_DIRECTORY = "src/fides"
 ADMIN_UI_DIRECTORY = "ui-build/static/admin/"
@@ -23,8 +24,12 @@ def get_package_path() -> Optional[Path]:
     return None
 
 
-def get_path_to_admin_ui_file(path: str) -> Optional[Path]:
-    """Return a path to a packaged admin UI file."""
+def get_path_to_admin_ui_file(path: str = "") -> Optional[Path]:
+    """
+    Return a path to a packaged admin UI file.
+
+    If no path is given, returns the path to the root of the UI directory
+    """
     package_path = get_package_path()
     if package_path is None:
         return None
@@ -127,6 +132,16 @@ def match_route(route_file_map: Dict[re.Pattern, Path], route: str) -> Optional[
     return sorted(matches)[0]
 
 
+def path_is_in_ui_directory(path: Path) -> bool:
+    """Checks if the path exists within the UI directory"""
+    ui_directory = get_path_to_admin_ui_file()
+    if not ui_directory:
+        logger.debug("Unable to locate UI directory")
+        return False
+
+    return ui_directory in path.parents
+
+
 def _is_dynamic_path(path: Path) -> bool:
     """Returns true if the given route is a dynamic NextJS route (e.g. "dataset/[id].html")"""
     return re.compile(r"\[\w+\]").search(str(path)) is not None

src/fides/api/main.py

@@ -37,6 +37,7 @@
     get_package_file_map,
     get_path_to_admin_ui_file,
     match_route,
+    path_is_in_ui_directory,
 )
 from fides.api.ctl.utils.errors import FidesError
 from fides.api.ctl.utils.logger import setup as setup_logging
@@ -311,8 +312,12 @@ def read_other_paths(request: Request) -> Response:
     if not ui_file:
         ui_file = get_path_to_admin_ui_file(path)
 
-    # If any of those worked, serve the file.
+    # Serve up the file as long as it is within the UI directory
     if ui_file and ui_file.is_file():
+        if not path_is_in_ui_directory(ui_file):
+            raise HTTPException(
+                status_code=status.HTTP_404_NOT_FOUND, detail="Item not found"
+            )
         logger.debug(
             "catchall request path '{}' matched static admin UI file: {}",
             path,

tests/ctl/api/test_ui.py

@@ -2,10 +2,18 @@
 import re
 from pathlib import Path
 from typing import Dict
+from unittest import mock
+from unittest.mock import Mock
 
 import pytest
+import requests
+from starlette.testclient import TestClient
 
-from fides.api.ctl.ui import generate_route_file_map, match_route
+from fides.api.ctl.ui import (
+    generate_route_file_map,
+    match_route,
+    path_is_in_ui_directory,
+)
 
 # Path segments of temporary files whose routes are tested.
 STATIC_FILES = (
@@ -63,3 +71,34 @@ def test_match_route(
 
     # Test example routes.
     assert match_route(route_file_map, route) == tmp_static / expected
+
+
+@pytest.mark.unit
+@mock.patch("fides.api.ctl.ui.get_path_to_admin_ui_file")
+@pytest.mark.parametrize(
+    "route, expected",
+    [
+        ("index.html", True),
+        ("//etc/passwd", False),
+        ("dataset/new.html", True),
+        ("//fides/example.env", False),
+    ],
+)
+def test_path_is_in_ui_directory(
+    mock_get_path_to_admin_ui_file: Mock, tmp_static: Path, route: str, expected: bool
+):
+    """Test various paths for if they are in the UI directory"""
+    mock_get_path_to_admin_ui_file.return_value = tmp_static
+    assert path_is_in_ui_directory(tmp_static / Path(route)) == expected
+
+
+@pytest.mark.integration
+@pytest.mark.parametrize("route, expected", [("/", 200), ("//etc/passwd", 404)])
+def test_check_file_within_ui_directory(
+    test_client: TestClient, route: str, expected: int
+):
+    """Test attempts at retrieving files outside the UI directory"""
+    # We use localhost:8080 here because otherwise TestClient will strip out
+    # the leading `//` for malicious paths
+    res = test_client.get(f"http://localhost:8080{route}")
+    assert res.status_code == expected