T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/itinerary_generator.py:384
- Finding
- Path Traversal Through Itinerary Output Filename<![CDATA[ ## Vulnerability Details **File Location**: `scripts/itinerary_generator.py`, lines 384-385 **Vulnerability Type**: Unsanitized user-controlled output path **Risk Level**: Medium ### Vulnerable Code ```python destination = sys.argv[1] duration = int(sys.argv[2]) trip_type = sys.argv[3] if len(sys.argv) > 3 else "romantic" generator = TravelItineraryGenerator(destination, duration, trip_type) itinerary = generator.generate_full_itinerary() # Save to file output_file = f"{destination.lower()}_itinerary_{duration}days.json" save_itinerary_to_file(itinerary, output_file) ``` The resulting path is passed to the following file-writing operation: ```python def save_itinerary_to_file(itinerary, output_file="travel_itinerary.json"): with open(output_file, 'w', encoding='utf-8') as f: json.dump(itinerary, f, indent=2, ensure_ascii=False) ``` ### Technical Analysis The `destination` command-line argument is incorporated directly into an output filename. The code does not reject absolute paths, path separators, `..` components, or other filesystem metacharacters. Because Python's `open(..., "w")` follows the constructed path and truncates an existing file, a destination containing traversal components can cause the itinerary to be written outside the intended working directory. The fixed `_itinerary_<duration>days.json` suffix limits which filenames can be targeted, but it does not confine writes to an approved directory. ### Attack Path 1. An attacker gains control over the destination argument, directly or through an application that invokes the script. 2. The attacker supplies a traversal value such as `../shared/report`. 3. The script constructs a path similar to: ```text ../shared/report_itinerary_5days.json ``` 4. `save_itinerary_to_file` opens the path in write mode. 5. A file outside the intended output directory is created or an existing matching file is overwritten. ### Impact Assessment Exploitation grants filesystem ...[truncated 476 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Store all generated files beneath a fixed output directory. 2. Convert the destination to a strict filename slug containing only approved characters, such as ASCII letters, digits, hyphens, and underscores. 3. Reject absolute paths, `..`, forward slashes, backslashes, NUL characters, and empty filenames. 4. Resolve the final path and verify that it remains beneath the approved output directory. 5. Avoid silently truncating existing files; use exclusive creation or require explicit overwrite approval. 6. Consider rejecting symbolic-link output targets. Example hardening: ```python import re from pathlib import Path OUTPUT_DIR = Path("generated_itineraries").resolve() OUTPUT_DIR.mkdir(parents=True, exist_ok=True) def safe_slug(value): slug = re.sub(r"[^A-Za-z0-9_-]+", "_", value).strip("_") if not slug: raise ValueError("Destination does not produce a valid filename") return slug safe_destination = safe_slug(destination) output_file = ( OUTPUT_DIR / f"{safe_destination}_itinerary_{duration}days.json" ).resolve() if OUTPUT_DIR not in output_file.parents: raise ValueError("Output path escapes the approved directory") ``` ]]>