From 8235b36afd79f6e70c3c3fc3455b564d175efc49 Mon Sep 17 00:00:00 2001 From: mARTin-B78 Date: Fri, 4 Sep 2026 08:04:26 +0200 Subject: [PATCH] security: validate upload filenames and archive paths --- CHANGELOG.md | 2 ++ TODO.md | 4 ++-- core/validation.py | 13 +++++++++++++ routes/conversation.py | 10 +++++++--- routes/library.py | 14 ++++---------- routes/stt.py | 10 +++------- tests/test_security.py | 8 +++++++- 7 files changed, 38 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a4a9dba..b0c7e5a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,8 @@ Follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) · versioned wi changing those fields causes the existing pair to be flagged stale. - **Character Table Pagination.** Productions now render 50 character cards/rows at a time with an explicit Show more control, keeping sorting and record lookups intact while reducing DOM and image work for large casts. +- **Upload Filename Boundaries.** Audio, STT, route-sound and picture uploads now reject path-like or control-character + filenames before deriving extensions; voice ZIP imports also reject absolute, traversal and malformed archive paths. - **Security Regression Coverage.** Added a Python test gate for character metadata validation, private-download URL rejection and oversized JSON request rejection. - **Navigation Reliability.** Rehearser phase requests now survive the feature-module initialization boundary. diff --git a/TODO.md b/TODO.md index 2b61a25..a67e214 100644 --- a/TODO.md +++ b/TODO.md @@ -20,8 +20,8 @@ the first block before investing in architecture or polish. - [ ] **S — Stop API keys from reaching frontend JavaScript or logs.** Provider keys are now masked in settings responses and probes use server-side credentials; finish the external-client key handoff and audit remaining transient input/error paths. -- [ ] **S — Validate uploaded filenames and paths.** Reject traversal, unsafe names and - writes outside the intended data directories. +- [x] **S — Validate uploaded filenames and paths.** Upload endpoints reject traversal, path separators and control + characters before deriving extensions; ZIP imports reject absolute, traversal and malformed archive members. - [ ] **M — Add upload, request-size, temporary-file and concurrent-job limits.** Apply endpoint-specific caps and clean up intermediates on failure. - [x] **M — Add automatic backups before bulk deletes, voice moves and recasts.** Make diff --git a/core/validation.py b/core/validation.py index a180777..d85012a 100644 --- a/core/validation.py +++ b/core/validation.py @@ -52,6 +52,19 @@ def _copy_limited(src, dest, limit: int) -> int: return total +def _validated_upload_suffix(filename: str | None, *, default: str, allowed: set[str]) -> str: + """Validate a browser upload name and return its normalized suffix.""" + name = str(filename or default) + if not name or len(name) > 255 or any(ord(char) < 32 or ord(char) == 127 for char in name): + raise HTTPException(400, "Invalid upload filename") + if "/" in name or "\\" in name or name in {".", ".."}: + raise HTTPException(400, "Upload filename must not contain a path") + suffix = Path(name).suffix.lower() or default + if suffix not in allowed: + raise HTTPException(400, "Unsupported file type") + return suffix + + def _safe_child_path(root: Path, candidate: Path) -> Path: root_resolved = root.resolve() candidate_resolved = candidate.resolve() diff --git a/routes/conversation.py b/routes/conversation.py index 2527e67..fbbd20d 100644 --- a/routes/conversation.py +++ b/routes/conversation.py @@ -3961,10 +3961,14 @@ async def voices_import(file: UploadFile = File(...)): for info in zf.infolist(): if info.is_dir(): continue - parts = Path(info.filename).parts - if any(p in ("..", "") for p in parts) or Path(info.filename).name == "settings.json": + archive_name = str(info.filename or "") + parts = tuple(archive_name.replace("\\", "/").split("/")) + if (not archive_name or archive_name.startswith(("/", "\\")) + or any(p in ("..", "") for p in parts) + or any(ord(char) < 32 or ord(char) == 127 for char in archive_name) + or Path(parts[-1]).name == "settings.json"): continue - if Path(info.filename).suffix.lower() not in _IMPORT_ALLOWED_SUFFIXES: + if Path(parts[-1]).suffix.lower() not in _IMPORT_ALLOWED_SUFFIXES: continue rel = parts[1:] if len(parts) > 1 else parts dest = scan_dir / Path(*rel) diff --git a/routes/library.py b/routes/library.py index 3b42efa..5ad42e5 100644 --- a/routes/library.py +++ b/routes/library.py @@ -14,7 +14,7 @@ from fastapi.responses import FileResponse, Response from core.config import _load_settings from core.constants import _VOICES_DIR_DEFAULT, _OUTPUT_DIR_DEFAULT, _VOICE_TARGET_DBFS, _VOICE_PEAK_DBFS, _BENCHMARK_SENTENCES, _MAX_TTS_OUTPUT_SECONDS, _MAX_UPLOAD_BYTES, _MAX_PICTURE_BYTES, _MAX_SOUND_BYTES, _ALLOW_PRIVATE_DOWNLOADS from core.registry import _registry_put, _registry_get, TEMP_DIR -from core.validation import _copy_limited, _safe_child_path, _validate_http_url +from core.validation import _copy_limited, _safe_child_path, _validate_http_url, _validated_upload_suffix from core.audio import ( _to_wav_24k, _to_wav_16k, _trim, _duration, _export_normalized_wav, _loudness_info, _auto_trim_bounds, _normalize_segment, @@ -147,9 +147,7 @@ async def save_book_profile(request: Request): @router.post("/api/upload") async def upload(file: UploadFile = File(...)): - suffix = Path(file.filename or "audio").suffix.lower() or ".bin" - if suffix not in _UPLOAD_EXTS: - raise HTTPException(400, "Unsupported audio/video file type") + suffix = _validated_upload_suffix(file.filename, default=".bin", allowed=_UPLOAD_EXTS) dest = TEMP_DIR / f"{uuid.uuid4().hex}{suffix}" try: with dest.open("wb") as f: @@ -227,9 +225,7 @@ async def route_sound_file(sound_path: str): async def upload_route_sound(file: UploadFile = File(...), name: str = Form("")): from pydub import AudioSegment - suffix = Path(file.filename or "sound").suffix.lower() or ".bin" - if suffix not in _AUDIO_EXTS: - raise HTTPException(400, "Unsupported sound file type") + suffix = _validated_upload_suffix(file.filename, default=".bin", allowed=_AUDIO_EXTS) raw_name = Path(name or file.filename or "sound").stem.strip() safe_name = re.sub(r"[^A-Za-z0-9_\-.]+", "_", raw_name).strip("._-")[:80] or f"sound_{uuid.uuid4().hex[:8]}" settings = _load_settings() @@ -945,9 +941,7 @@ async def upload_picture(voice_id: str = Form(...), file: UploadFile = File(...) if wav is None: raise HTTPException(404, f"Voice '{voice_id}' not found") - orig_suffix = Path(file.filename or "photo.jpg").suffix.lower() - if orig_suffix not in _PICTURE_EXTS: - orig_suffix = ".jpg" + orig_suffix = _validated_upload_suffix(file.filename, default=".jpg", allowed=_PICTURE_EXTS) _backup_existing_picture(wav) for ext in _PICTURE_EXTS: diff --git a/routes/stt.py b/routes/stt.py index e754793..6e18fd7 100644 --- a/routes/stt.py +++ b/routes/stt.py @@ -19,7 +19,7 @@ from core.constants import ( _MAX_UPLOAD_BYTES, _STT_REQUEST_TIMEOUT, ) from core.registry import _registry_get, TEMP_DIR -from core.validation import _validate_http_url, _copy_limited +from core.validation import _validate_http_url, _copy_limited, _validated_upload_suffix from core.audio import _to_wav_16k from core.voice import _AUDIO_EXTS, _UPLOAD_EXTS @@ -506,9 +506,7 @@ async def transcribe_bytes( backend: str = Form("configured"), ): """Accept raw audio upload and return transcription directly (used by hotkey daemon).""" - suffix = Path(file.filename or "audio.wav").suffix.lower() or ".wav" - if suffix not in _AUDIO_EXTS: - raise HTTPException(400, "Unsupported audio type") + suffix = _validated_upload_suffix(file.filename, default=".wav", allowed=_AUDIO_EXTS) tmp = TEMP_DIR / f"{uuid.uuid4().hex}_daemon{suffix}" wav_tmp = tmp try: @@ -554,9 +552,7 @@ async def stt_benchmark( else: if audio is None: raise HTTPException(400, "Audio file or voice library sample is required") - suffix = Path(audio.filename or "audio.wav").suffix.lower() or ".wav" - if suffix not in (_AUDIO_EXTS | _UPLOAD_EXTS): - raise HTTPException(400, "Unsupported audio type") + suffix = _validated_upload_suffix(audio.filename, default=".wav", allowed=(_AUDIO_EXTS | _UPLOAD_EXTS)) tmp = TEMP_DIR / f"{uuid.uuid4().hex}_stt_bench{suffix}" wav_tmp = tmp with tmp.open("wb") as f: diff --git a/tests/test_security.py b/tests/test_security.py index f58c79b..645bae3 100644 --- a/tests/test_security.py +++ b/tests/test_security.py @@ -10,7 +10,7 @@ os.environ.setdefault("CONFIG_DIR", tempfile.mkdtemp(prefix="ttvc-test-config-") from fastapi import HTTPException -from core.validation import _validate_character_record, _validate_http_url +from core.validation import _validate_character_record, _validate_http_url, _validated_upload_suffix from core.config import _load_settings, _save_settings from server import RequestSizeLimitMiddleware @@ -32,6 +32,12 @@ class SecurityValidationTests(unittest.TestCase): with self.assertRaises(HTTPException): _validate_http_url("http://127.0.0.1:8188/image", allow_private=False) + def test_upload_filename_rejects_paths_and_control_characters(self): + for filename in ("../voice.wav", r"folder\\voice.wav", "voice\x00.wav"): + with self.assertRaises(HTTPException): + _validated_upload_suffix(filename, default=".wav", allowed={".wav"}) + self.assertEqual(_validated_upload_suffix("VOICE.WAV", default=".wav", allowed={".wav"}), ".wav") + def test_request_size_middleware_rejects_oversized_json(self): sent = []