security: validate upload filenames and archive paths
This commit is contained in:
parent
b5d2e773d8
commit
8235b36afd
@ -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.
|
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
|
- **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.
|
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
|
- **Security Regression Coverage.** Added a Python test gate for character metadata validation, private-download URL
|
||||||
rejection and oversized JSON request rejection.
|
rejection and oversized JSON request rejection.
|
||||||
- **Navigation Reliability.** Rehearser phase requests now survive the feature-module initialization boundary.
|
- **Navigation Reliability.** Rehearser phase requests now survive the feature-module initialization boundary.
|
||||||
|
|||||||
4
TODO.md
4
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
|
- [ ] **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
|
responses and probes use server-side credentials; finish the external-client key handoff and audit remaining
|
||||||
transient input/error paths.
|
transient input/error paths.
|
||||||
- [ ] **S — Validate uploaded filenames and paths.** Reject traversal, unsafe names and
|
- [x] **S — Validate uploaded filenames and paths.** Upload endpoints reject traversal, path separators and control
|
||||||
writes outside the intended data directories.
|
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
|
- [ ] **M — Add upload, request-size, temporary-file and concurrent-job limits.** Apply
|
||||||
endpoint-specific caps and clean up intermediates on failure.
|
endpoint-specific caps and clean up intermediates on failure.
|
||||||
- [x] **M — Add automatic backups before bulk deletes, voice moves and recasts.** Make
|
- [x] **M — Add automatic backups before bulk deletes, voice moves and recasts.** Make
|
||||||
|
|||||||
@ -52,6 +52,19 @@ def _copy_limited(src, dest, limit: int) -> int:
|
|||||||
return total
|
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:
|
def _safe_child_path(root: Path, candidate: Path) -> Path:
|
||||||
root_resolved = root.resolve()
|
root_resolved = root.resolve()
|
||||||
candidate_resolved = candidate.resolve()
|
candidate_resolved = candidate.resolve()
|
||||||
|
|||||||
@ -3961,10 +3961,14 @@ async def voices_import(file: UploadFile = File(...)):
|
|||||||
for info in zf.infolist():
|
for info in zf.infolist():
|
||||||
if info.is_dir():
|
if info.is_dir():
|
||||||
continue
|
continue
|
||||||
parts = Path(info.filename).parts
|
archive_name = str(info.filename or "")
|
||||||
if any(p in ("..", "") for p in parts) or Path(info.filename).name == "settings.json":
|
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
|
continue
|
||||||
if Path(info.filename).suffix.lower() not in _IMPORT_ALLOWED_SUFFIXES:
|
if Path(parts[-1]).suffix.lower() not in _IMPORT_ALLOWED_SUFFIXES:
|
||||||
continue
|
continue
|
||||||
rel = parts[1:] if len(parts) > 1 else parts
|
rel = parts[1:] if len(parts) > 1 else parts
|
||||||
dest = scan_dir / Path(*rel)
|
dest = scan_dir / Path(*rel)
|
||||||
|
|||||||
@ -14,7 +14,7 @@ from fastapi.responses import FileResponse, Response
|
|||||||
from core.config import _load_settings
|
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.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.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 (
|
from core.audio import (
|
||||||
_to_wav_24k, _to_wav_16k, _trim, _duration, _export_normalized_wav,
|
_to_wav_24k, _to_wav_16k, _trim, _duration, _export_normalized_wav,
|
||||||
_loudness_info, _auto_trim_bounds, _normalize_segment,
|
_loudness_info, _auto_trim_bounds, _normalize_segment,
|
||||||
@ -147,9 +147,7 @@ async def save_book_profile(request: Request):
|
|||||||
|
|
||||||
@router.post("/api/upload")
|
@router.post("/api/upload")
|
||||||
async def upload(file: UploadFile = File(...)):
|
async def upload(file: UploadFile = File(...)):
|
||||||
suffix = Path(file.filename or "audio").suffix.lower() or ".bin"
|
suffix = _validated_upload_suffix(file.filename, default=".bin", allowed=_UPLOAD_EXTS)
|
||||||
if suffix not in _UPLOAD_EXTS:
|
|
||||||
raise HTTPException(400, "Unsupported audio/video file type")
|
|
||||||
dest = TEMP_DIR / f"{uuid.uuid4().hex}{suffix}"
|
dest = TEMP_DIR / f"{uuid.uuid4().hex}{suffix}"
|
||||||
try:
|
try:
|
||||||
with dest.open("wb") as f:
|
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("")):
|
async def upload_route_sound(file: UploadFile = File(...), name: str = Form("")):
|
||||||
from pydub import AudioSegment
|
from pydub import AudioSegment
|
||||||
|
|
||||||
suffix = Path(file.filename or "sound").suffix.lower() or ".bin"
|
suffix = _validated_upload_suffix(file.filename, default=".bin", allowed=_AUDIO_EXTS)
|
||||||
if suffix not in _AUDIO_EXTS:
|
|
||||||
raise HTTPException(400, "Unsupported sound file type")
|
|
||||||
raw_name = Path(name or file.filename or "sound").stem.strip()
|
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]}"
|
safe_name = re.sub(r"[^A-Za-z0-9_\-.]+", "_", raw_name).strip("._-")[:80] or f"sound_{uuid.uuid4().hex[:8]}"
|
||||||
settings = _load_settings()
|
settings = _load_settings()
|
||||||
@ -945,9 +941,7 @@ async def upload_picture(voice_id: str = Form(...), file: UploadFile = File(...)
|
|||||||
if wav is None:
|
if wav is None:
|
||||||
raise HTTPException(404, f"Voice '{voice_id}' not found")
|
raise HTTPException(404, f"Voice '{voice_id}' not found")
|
||||||
|
|
||||||
orig_suffix = Path(file.filename or "photo.jpg").suffix.lower()
|
orig_suffix = _validated_upload_suffix(file.filename, default=".jpg", allowed=_PICTURE_EXTS)
|
||||||
if orig_suffix not in _PICTURE_EXTS:
|
|
||||||
orig_suffix = ".jpg"
|
|
||||||
|
|
||||||
_backup_existing_picture(wav)
|
_backup_existing_picture(wav)
|
||||||
for ext in _PICTURE_EXTS:
|
for ext in _PICTURE_EXTS:
|
||||||
|
|||||||
@ -19,7 +19,7 @@ from core.constants import (
|
|||||||
_MAX_UPLOAD_BYTES, _STT_REQUEST_TIMEOUT,
|
_MAX_UPLOAD_BYTES, _STT_REQUEST_TIMEOUT,
|
||||||
)
|
)
|
||||||
from core.registry import _registry_get, TEMP_DIR
|
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.audio import _to_wav_16k
|
||||||
from core.voice import _AUDIO_EXTS, _UPLOAD_EXTS
|
from core.voice import _AUDIO_EXTS, _UPLOAD_EXTS
|
||||||
|
|
||||||
@ -506,9 +506,7 @@ async def transcribe_bytes(
|
|||||||
backend: str = Form("configured"),
|
backend: str = Form("configured"),
|
||||||
):
|
):
|
||||||
"""Accept raw audio upload and return transcription directly (used by hotkey daemon)."""
|
"""Accept raw audio upload and return transcription directly (used by hotkey daemon)."""
|
||||||
suffix = Path(file.filename or "audio.wav").suffix.lower() or ".wav"
|
suffix = _validated_upload_suffix(file.filename, default=".wav", allowed=_AUDIO_EXTS)
|
||||||
if suffix not in _AUDIO_EXTS:
|
|
||||||
raise HTTPException(400, "Unsupported audio type")
|
|
||||||
tmp = TEMP_DIR / f"{uuid.uuid4().hex}_daemon{suffix}"
|
tmp = TEMP_DIR / f"{uuid.uuid4().hex}_daemon{suffix}"
|
||||||
wav_tmp = tmp
|
wav_tmp = tmp
|
||||||
try:
|
try:
|
||||||
@ -554,9 +552,7 @@ async def stt_benchmark(
|
|||||||
else:
|
else:
|
||||||
if audio is None:
|
if audio is None:
|
||||||
raise HTTPException(400, "Audio file or voice library sample is required")
|
raise HTTPException(400, "Audio file or voice library sample is required")
|
||||||
suffix = Path(audio.filename or "audio.wav").suffix.lower() or ".wav"
|
suffix = _validated_upload_suffix(audio.filename, default=".wav", allowed=(_AUDIO_EXTS | _UPLOAD_EXTS))
|
||||||
if suffix not in (_AUDIO_EXTS | _UPLOAD_EXTS):
|
|
||||||
raise HTTPException(400, "Unsupported audio type")
|
|
||||||
tmp = TEMP_DIR / f"{uuid.uuid4().hex}_stt_bench{suffix}"
|
tmp = TEMP_DIR / f"{uuid.uuid4().hex}_stt_bench{suffix}"
|
||||||
wav_tmp = tmp
|
wav_tmp = tmp
|
||||||
with tmp.open("wb") as f:
|
with tmp.open("wb") as f:
|
||||||
|
|||||||
@ -10,7 +10,7 @@ os.environ.setdefault("CONFIG_DIR", tempfile.mkdtemp(prefix="ttvc-test-config-")
|
|||||||
|
|
||||||
from fastapi import HTTPException
|
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 core.config import _load_settings, _save_settings
|
||||||
from server import RequestSizeLimitMiddleware
|
from server import RequestSizeLimitMiddleware
|
||||||
|
|
||||||
@ -32,6 +32,12 @@ class SecurityValidationTests(unittest.TestCase):
|
|||||||
with self.assertRaises(HTTPException):
|
with self.assertRaises(HTTPException):
|
||||||
_validate_http_url("http://127.0.0.1:8188/image", allow_private=False)
|
_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):
|
def test_request_size_middleware_rejects_oversized_json(self):
|
||||||
sent = []
|
sent = []
|
||||||
|
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user