T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/protein_key_fragment_analysis.py:423
- Finding
- Path Traversal Through Unsanitized Species Name Allows Writes Outside the Output Directory<![CDATA[ ## Vulnerability Details **File Location**: `scripts/protein_key_fragment_analysis.py`, lines 423–484 **Vulnerability Type**: Path traversal / arbitrary-location file write **Risk Level**: High ### Complete Vulnerable Code Snippet ```python species_dir = Path(output_dir) / species_name species_dir.mkdir(exist_ok=True) # ... aln_path = species_dir / f"{species_name}_aligned.fasta" # ... consensus_path = species_dir / f"{species_name}_consensus.fasta" clean_consensus = "".join(aa for aa in consensus_seq if aa != '-') with open(consensus_path, 'w') as f: f.write(f">{species_name}_consensus\n") for i in range(0, len(clean_consensus), 60): f.write(clean_consensus[i:i+60] + "\n") # ... fragments_path = species_dir / f"{species_name}_key_fragments.json" with open(fragments_path, 'w', encoding='utf-8') as f: json.dump({ "species": species_name, "date": DATE_STR, "consensus_length": len(clean_consensus), "key_fragments": key_fragments }, f, ensure_ascii=False, indent=2) report_path = generate_report( species_name, sequences, consensus_seq, conservation_scores, key_fragments, species_dir ) ``` The value reaches this function directly from the command-line positional argument: ```python parser.add_argument("species_name", nargs="?", help="物种名称") # ... result = analyze_species(args.species_name, fasta, out_dir, precomputed_aligned=args.precomputed_aligned) ``` ### Technical Analysis The user-controlled `species_name` is used as both a directory component and part of several output filenames without validation. `pathlib.Path` does not automatically constrain a joined path to its intended base directory. A value containing parent-directory components, such as `../target`, can escape `output_dir`. An absolute value can also cause the preceding base path to be discarded during path composition. The resulting paths are subsequently passed to directory creation, Py ...[truncated 2723 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Treat the species name as a display label, not as a filesystem path. 2. Convert it to a strict filename-safe identifier. Prefer an allowlist such as ASCII letters, digits, underscores, and hyphens. 3. Reject absolute paths, `.` and `..` components, path separators, control characters, and empty identifiers. 4. Resolve every output path and verify that it remains beneath the resolved output root before creating or writing it. 5. Use the sanitized identifier consistently for all directories and filenames while preserving the original species name only inside report content. 6. Avoid running this analysis with elevated privileges. 7. Add tests for traversal values, absolute paths, mixed separators, and encoded or Unicode separator-like characters. A hardened implementation could use: ```python import re from pathlib import Path def safe_species_id(species_name: str) -> str: if not re.fullmatch(r"[A-Za-z0-9_-]+", species_name): raise ValueError( "Species name must contain only letters, digits, underscores, and hyphens" ) return species_name def confined_path(base: Path, *parts: str) -> Path: base = base.resolve() candidate = base.joinpath(*parts).resolve() if candidate != base and base not in candidate.parents: raise ValueError("Output path escapes the configured output directory") return candidate species_id = safe_species_id(species_name) output_root = Path(output_dir).resolve() output_root.mkdir(parents=True, exist_ok=True) species_dir = confined_path(output_root, species_id) species_dir.mkdir(parents=False, exist_ok=True) aln_path = confined_path( species_dir, f"{species_id}_aligned.fasta" ) consensus_path = confined_path( species_dir, f"{species_id}_consensus.fasta" ) fragments_path = confined_path( species_dir, f"{species_id}_key_fragments.json" ) ``` If spaces or non-ASCII biological names must be supported, create a deterministic safe slug or ...[truncated 163 chars]