diff --git a/backend/ten31portal/main.py b/backend/ten31portal/main.py index 7d82653..c1dbdc9 100644 --- a/backend/ten31portal/main.py +++ b/backend/ten31portal/main.py @@ -57,6 +57,19 @@ def health() -> dict[str, str]: return {"status": "ok"} +def _contained_static_path(static_root: Path, path: str) -> Path | None: + """Resolve `path` under `static_root`, returning the file only if it stays + within the root. Percent-encoded traversal (..%2f) survives routing and + would otherwise let an unauthenticated caller read files outside static/ + (e.g. the DB or session secret on the data volume). Returns None if the + resolved path escapes the root.""" + root = static_root.resolve() + candidate = (root / path).resolve() + if candidate != root and root not in candidate.parents: + return None + return candidate + + # Serve built frontend in production (when static/ dir exists next to the app) _static_dir = Path(__file__).resolve().parent.parent / "static" if _static_dir.is_dir(): @@ -72,7 +85,9 @@ if _static_dir.is_dir(): @app.api_route("/{path:path}", methods=["GET", "HEAD"]) async def serve_spa(path: str): - file = _static_dir / path + file = _contained_static_path(_static_dir, path) + if file is None: + return _index() if file.is_file(): # Don't let the HTML entrypoint get cached; fingerprinted assets can cache. if file.name == "index.html": diff --git a/backend/tests/test_static_traversal.py b/backend/tests/test_static_traversal.py new file mode 100644 index 0000000..ef7783d --- /dev/null +++ b/backend/tests/test_static_traversal.py @@ -0,0 +1,68 @@ +"""Regression test for the SPA static-file path-traversal fix. + +Before the fix, the `/{path:path}` catch-all joined the request path onto the +static dir with no containment check, so percent-encoded traversal +(GET /..%2f..%2fdata%2fportal.db) read arbitrary files off disk — including the +session secret, which allowed forging an admin session. See main.py. +""" + +import tempfile +from pathlib import Path + +from fastapi import FastAPI +from fastapi.responses import FileResponse +from fastapi.testclient import TestClient + +from ten31portal.main import _contained_static_path + + +def test_contained_helper_blocks_traversal(): + root = Path(tempfile.mkdtemp()) + static = root / "static" + static.mkdir() + (static / "index.html").write_text("spa") + (static / "app.js").write_text("ok") + data = root / "data" + data.mkdir() + (data / "portal.db").write_text("secret-db") + + # Legit assets resolve within the root. + assert _contained_static_path(static, "app.js") == (static / "app.js").resolve() + assert _contained_static_path(static, "index.html") == (static / "index.html").resolve() + + # Traversal (already-decoded, i.e. what ..%2f becomes) escapes -> None. + for evil in ("../data/portal.db", "../../data/portal.db", "../data/../data/portal.db"): + assert _contained_static_path(static, evil) is None, evil + + +def test_spa_route_does_not_leak_via_encoded_traversal(): + """End-to-end: encoded traversal against the real route shape returns the + SPA shell, never the out-of-root file.""" + root = Path(tempfile.mkdtemp()) + static = root / "static" + static.mkdir() + (static / "index.html").write_text("SPA") + data = root / "data" + data.mkdir() + (data / ".session-secret").write_text("TOPSECRET") + + app = FastAPI() + + def _index(): + return FileResponse(static / "index.html") + + @app.api_route("/{path:path}", methods=["GET", "HEAD"]) + async def serve_spa(path: str): + file = _contained_static_path(static, path) + if file is None or not file.is_file(): + return _index() + return FileResponse(file) + + client = TestClient(app) + for attack in ( + "/..%2f..%2fdata%2f.session-secret", + "/%2e%2e%2f%2e%2e%2fdata%2f.session-secret", + "/../../data/.session-secret", + ): + r = client.get(attack) + assert "TOPSECRET" not in r.text, attack diff --git a/deploy/startos/install/versions/index.ts b/deploy/startos/install/versions/index.ts index 87ca3d9..bb3910c 100644 --- a/deploy/startos/install/versions/index.ts +++ b/deploy/startos/install/versions/index.ts @@ -1,5 +1,6 @@ -export { v_0_2_40 as current } from './v_0_2_40' +export { v_0_2_41 as current } from './v_0_2_41' import { v_0_1_0 } from './v_0_1_0' +import { v_0_2_40 } from './v_0_2_40' import { v_0_2_0 } from './v_0_2_0' import { v_0_2_1 } from './v_0_2_1' import { v_0_2_3 } from './v_0_2_3' @@ -39,4 +40,4 @@ import { v_0_2_36 } from './v_0_2_36' import { v_0_2_37 } from './v_0_2_37' import { v_0_2_38 } from './v_0_2_38' import { v_0_2_39 } from './v_0_2_39' -export const other = [v_0_1_0, v_0_2_0, v_0_2_1, v_0_2_3, v_0_2_4, v_0_2_5, v_0_2_6, v_0_2_7, v_0_2_8, v_0_2_9, v_0_2_10, v_0_2_11, v_0_2_12, v_0_2_13, v_0_2_14, v_0_2_15, v_0_2_16, v_0_2_17, v_0_2_18, v_0_2_19, v_0_2_20, v_0_2_21, v_0_2_22, v_0_2_23, v_0_2_24, v_0_2_25, v_0_2_26, v_0_2_27, v_0_2_28, v_0_2_29, v_0_2_30, v_0_2_31, v_0_2_32, v_0_2_33, v_0_2_34, v_0_2_35, v_0_2_36, v_0_2_37, v_0_2_38, v_0_2_39] +export const other = [v_0_1_0, v_0_2_0, v_0_2_1, v_0_2_3, v_0_2_4, v_0_2_5, v_0_2_6, v_0_2_7, v_0_2_8, v_0_2_9, v_0_2_10, v_0_2_11, v_0_2_12, v_0_2_13, v_0_2_14, v_0_2_15, v_0_2_16, v_0_2_17, v_0_2_18, v_0_2_19, v_0_2_20, v_0_2_21, v_0_2_22, v_0_2_23, v_0_2_24, v_0_2_25, v_0_2_26, v_0_2_27, v_0_2_28, v_0_2_29, v_0_2_30, v_0_2_31, v_0_2_32, v_0_2_33, v_0_2_34, v_0_2_35, v_0_2_36, v_0_2_37, v_0_2_38, v_0_2_39, v_0_2_40] diff --git a/deploy/startos/install/versions/v_0_2_41.ts b/deploy/startos/install/versions/v_0_2_41.ts new file mode 100644 index 0000000..b489ae2 --- /dev/null +++ b/deploy/startos/install/versions/v_0_2_41.ts @@ -0,0 +1,17 @@ +import { VersionInfo } from '@start9labs/start-sdk' + +export const v_0_2_41 = VersionInfo.of({ + version: '0.2.41:0', + releaseNotes: { + en_US: + 'Security fix: closes an unauthenticated path-traversal flaw in the ' + + 'static file server that let a crafted URL read files outside the web ' + + 'root (including the database and session secret). Requests are now ' + + 'contained to the frontend build directory. Upgrade recommended for all ' + + 'internet-facing deployments.', + }, + migrations: { + up: async ({ effects }) => {}, + down: async ({ effects }) => {}, + }, +})