From 3a7b0fed492eeed09bc45ae6b2a03a82d7d1bf2d Mon Sep 17 00:00:00 2001 From: swizzcheeze <13037010+swizzcheeze@users.noreply.github.com> Date: Tue, 17 Mar 2026 01:06:56 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=92=20[security=20fix]=20Fix=20path=20?= =?UTF-8?q?traversal=20in=20model=20download=20path=20construction?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduced path validation using `pathlib.Path.resolve()` to ensure that constructed download paths for models and files remain within the intended base directory. This fixes a vulnerability where malformed `model_id` or `filename` inputs could be used to perform path traversal and write files outside of the designated storage area. Changes: - Added `validate_path` helper function to both `Hugging Hugger.py` and `CLI HUG.py`. - Integrated validation into single-file and entire-model download flows. - Added clear error reporting for blocked traversal attempts. - Improved path construction consistency in `CLI HUG.py`. --- CLI HUG.py | 29 ++++++++++++++++++++++++----- Hugging Hugger.py | 22 +++++++++++++++++++++- 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/CLI HUG.py b/CLI HUG.py index d629cdd..d0efe43 100644 --- a/CLI HUG.py +++ b/CLI HUG.py @@ -89,6 +89,14 @@ def update(self, *args, **kwargs): print(args[0] if args else "Working...") # --- Helper Functions --- +def validate_path(base_dir: Path, user_input_path: str) -> Path: + """Validates that the combined path remains within the base directory.""" + base = base_dir.resolve() + target = (base / user_input_path).resolve() + if not str(target).startswith(str(base)): + raise ValueError(f"Security Alert: Path traversal attempt detected! Path: {user_input_path}") + return target + def display_main_menu(): """Prints the main menu options.""" console.print(Panel( @@ -147,6 +155,13 @@ def run_single_download(): local_dir_str = Prompt.ask(f"[cyan]Enter Save Directory[/cyan]", default=str(DEFAULT_SAVE_DIR)) local_dir = Path(local_dir_str) + try: + validate_path(local_dir, repo_id) + validate_path(local_dir, filename) + except ValueError as ve: + console.print(f"[bold red]Error:[/bold red] {ve}") + return + if not ensure_directory(local_dir): return console.print(f"\n[magenta]Starting download...[/magenta]") @@ -166,8 +181,8 @@ def run_single_download(): console.rule() # Applying user's color preference for success message console.print(f"[#ADFF2F]Success![/#ADFF2F] File downloaded to:") - # Applying user's color preference for path and FIXING the closing tag - console.print(f" [bold #F59E0B]{file_path}[/bold #F59E0B]") # <-- FIX HERE + # Applying user's color preference for path + console.print(f" [bold #F59E0B]{file_path}[/bold #F59E0B]") except Exception as e: console.rule() @@ -186,6 +201,12 @@ def run_model_download(): if not ensure_directory(local_dir_base): return + try: + model_target_dir = validate_path(local_dir_base, repo_id) + except ValueError as ve: + console.print(f"[bold red]Error:[/bold red] {ve}") + return + # --- Get File Count --- num_files_str = "" try: @@ -202,11 +223,9 @@ def run_model_download(): console.print(f" Repo ID: [bold]{repo_id}[/bold]") # Keeping original bold for contrast console.print(f" Save Dir Base: [bright_green]{local_dir_base}[/bright_green]") # FIXING closing tag typo here - console.print(f" Workers: [bright_green]{workers}[/bright_green]") # <-- FIX HERE + console.print(f" Workers: [bright_green]{workers}[/bright_green]") console.rule(f"Starting Download{num_files_str}") - model_target_dir = local_dir_base / repo_id - try: model_path = snapshot_download( repo_id=repo_id, diff --git a/Hugging Hugger.py b/Hugging Hugger.py index 9ae0d1b..1bc5ab0 100644 --- a/Hugging Hugger.py +++ b/Hugging Hugger.py @@ -3,6 +3,7 @@ from tkinter import filedialog, messagebox, scrolledtext, Menu import tkinter.font as tkFont import os +from pathlib import Path import threading import queue import sys @@ -46,6 +47,14 @@ root_check.destroy(); sys.exit(1) # --- Threaded Download Functions --- +def validate_path(base_dir: str, user_input_path: str) -> Path: + """Validates that the combined path remains within the base directory.""" + base = Path(base_dir).resolve() + target = (base / user_input_path).resolve() + if not str(target).startswith(str(base)): + raise ValueError(f"Security Alert: Path traversal attempt detected! Path: {user_input_path}") + return target + # Modified slightly to check cancellation flag (though cannot interrupt mid-download) def download_single_file_threaded(model_id, filename, local_dir, status_queue, cancel_event): """Downloads a single file in a thread and reports status via queue.""" @@ -53,6 +62,13 @@ def download_single_file_threaded(model_id, filename, local_dir, status_queue, c file_path = None # Initialize file_path try: if cancel_event.is_set(): raise InterruptedError("Download cancelled before start") # Check before starting + + # Security validation for paths + validate_path(local_dir, model_id) + # Note: filename might contain subdirectories, validate it too if needed, + # but usually hf_hub_download handles it. For extra safety: + validate_path(local_dir, filename) + status_queue.put(f"Starting download: {filename} from {model_id}...") os.makedirs(local_dir, exist_ok=True) @@ -89,8 +105,12 @@ def download_entire_model_threaded(model_id, local_dir_base, num_workers, status model_path = None # Initialize try: if cancel_event.is_set(): raise InterruptedError("Download cancelled before start") + + # Security validation for path construction + model_target_dir = validate_path(local_dir_base, model_id) + status_queue.put(f"Starting download of entire model: {model_id} using {num_workers} workers...") - model_target_dir = os.path.join(local_dir_base, model_id); os.makedirs(model_target_dir, exist_ok=True) + os.makedirs(model_target_dir, exist_ok=True) # NOTE: snapshot_download itself cannot be easily interrupted by the flag here. model_path = snapshot_download(