0.2.41: contain SPA static serving to the web root
Percent-encoded traversal (..%2f) survived routing and let an unauthenticated request read files outside static/, including the database and session secret on the data volume. Paths are now resolved and contained to the frontend build directory; anything that escapes falls back to index.html.
This commit is contained in:
Jonathan Kirkwood
@@ -57,6 +57,19 @@ def health() -> dict[str, str]:
|
|||||||
return {"status": "ok"}
|
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)
|
# Serve built frontend in production (when static/ dir exists next to the app)
|
||||||
_static_dir = Path(__file__).resolve().parent.parent / "static"
|
_static_dir = Path(__file__).resolve().parent.parent / "static"
|
||||||
if _static_dir.is_dir():
|
if _static_dir.is_dir():
|
||||||
@@ -72,7 +85,9 @@ if _static_dir.is_dir():
|
|||||||
|
|
||||||
@app.api_route("/{path:path}", methods=["GET", "HEAD"])
|
@app.api_route("/{path:path}", methods=["GET", "HEAD"])
|
||||||
async def serve_spa(path: str):
|
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():
|
if file.is_file():
|
||||||
# Don't let the HTML entrypoint get cached; fingerprinted assets can cache.
|
# Don't let the HTML entrypoint get cached; fingerprinted assets can cache.
|
||||||
if file.name == "index.html":
|
if file.name == "index.html":
|
||||||
|
|||||||
@@ -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("<html>SPA</html>")
|
||||||
|
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
|
||||||
@@ -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_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_0 } from './v_0_2_0'
|
||||||
import { v_0_2_1 } from './v_0_2_1'
|
import { v_0_2_1 } from './v_0_2_1'
|
||||||
import { v_0_2_3 } from './v_0_2_3'
|
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_37 } from './v_0_2_37'
|
||||||
import { v_0_2_38 } from './v_0_2_38'
|
import { v_0_2_38 } from './v_0_2_38'
|
||||||
import { v_0_2_39 } from './v_0_2_39'
|
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]
|
||||||
|
|||||||
@@ -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 }) => {},
|
||||||
|
},
|
||||||
|
})
|
||||||
Reference in New Issue
Block a user