fix: path traversal in finetune Gradio handlers#1294
Conversation
Add _safe_project_path() helper that rejects absolute paths, null bytes, and path separators, then verifies the resolved path stays within the intended base directory via realpath + startswith check. Apply to all 10 sinks in save_settings, load_settings, create_data_project, vocab_extend, transcribe_all, create_metadata, and related functions.
|
Hi @niknah @v3ucn @karimouda , I opened a PR for this vulnerability. |
There was a problem hiding this comment.
Pull request overview
This PR addresses a path traversal vulnerability in the finetune Gradio handlers where attacker-controlled project names were previously used in os.path.join(base, project_name) without containment checks, allowing writes outside the intended base directories.
Changes:
- Added
_safe_project_path(base, name)to validate and contain project paths under a specified base directory. - Replaced multiple
os.path.join(path_data|path_project_ckpts, <user-input>)usages with_safe_project_path(...)across several finetune handlers (e.g., settings save/load, project creation, transcription, metadata creation, vocab ops, training path selection).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| is absolute, contains a null byte, or resolves outside base.""" | ||
| if not name or os.path.isabs(name) or "\x00" in name: | ||
| raise ValueError(f"invalid project_name: {name!r}") | ||
| # Strip path separators and control characters to a plain filename component. | ||
| name = re.sub(r"[/\\]", "", name).strip() | ||
| if not name or name in (".", ".."): | ||
| raise ValueError(f"invalid project_name: {name!r}") | ||
| candidate = os.path.realpath(os.path.join(base, name)) | ||
| base_real = os.path.realpath(base) | ||
| if not (candidate + os.sep).startswith(base_real + os.sep): |
| raise ValueError(f"invalid project_name: {name!r}") | ||
| candidate = os.path.realpath(os.path.join(base, name)) | ||
| base_real = os.path.realpath(base) | ||
| if not (candidate + os.sep).startswith(base_real + os.sep): |
|
@AAtomical thanks for PR~ The finetuning script is usually not hosted (also app not shared) and is executed by the user themselves. |
|
@SWivid Thanks for taking a look! I agree this does not grant privileges beyond the OS user running the finetuning script, and I’m not claiming privilege escalation or direct RCE. And the original PR is #1293 My concern is only the app-level directory boundary: user-controlled project names are expected to stay under the F5-TTS checkpoint directories, but absolute paths can escape those bases through os.path.join(). The PoC shows directory creation and a fixed-name setting.json write outside the intended project directory. I agree the severity is deployment-dependent and low when the UI is only run locally by a single trusted user. This is mainly a hardening PR to prevent writes outside the intended F5-TTS workspace. Thank you again!! |
Summary
Fixes #1293
save_settings,create_data_project, and related finetune handlers pass an attacker-controlledproject_namedirectly intoos.path.join(base, project_name)without any containment check. Python's documented behaviour: if the second argument is absolute, the base is silently discarded. A single Gradio API call withproject_name="/tmp/EVIL"writes files outside the intended directory.Changes
Added
_safe_project_path(base, name)helper that:/and\viare.sub.os.path.realpathand asserts the result starts withrealpath(base) + os.sep.Applied to all 10 path-join sinks across
save_settings,load_settings,create_data_project,vocab_extend,transcribe_all,create_metadata, and related functions.Test
Verified with end-to-end PoC (real
gradio_clientHTTP/WS RPC against a live Gradio server):ValueErrornot raised, files written to/tmp/F5TTS_PWND_pinyin/outside base.ValueError: invalid project_name: '/tmp/F5TTS_PWND_pinyin'raised server-side; no files written outside base.