T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/query_faers.py:73
- Finding
- User-Controlled Path Traversal and File Overwrite in FAERS Output Generation## Vulnerability Details **File Location**: `scripts/query_faers.py`, lines 73–156 **Vulnerability Type**: Path traversal and arbitrary file overwrite **Risk Level**: High The user-controlled `--drug` argument is incorporated into output filenames without filename sanitization or verification that the resulting paths remain inside the requested output directory. ### Vulnerable Code ```python os.makedirs(args.output, exist_ok=True) drug_name = args.drug.strip() # Resolve SMILES if looks like one if re.search(r'^[A-Za-z0-9#\(\)=\[\]@+\-%/.]+$', drug_name) and len(drug_name) > 3 and not drug_name.isalpha(): print(f"Resolving SMILES '{drug_name}' to drug name...", file=sys.stderr) resolved = get_drug_name_from_smiles(drug_name) if resolved: drug_name = resolved print(f"Resolved to: {drug_name}", file=sys.stderr) else: print(f"Failed to resolve SMILES '{args.drug}', using as-is.", file=sys.stderr) ``` The unsanitized value is subsequently used in multiple output paths: ```python yearly_counts.to_json( os.path.join(args.output, f'{drug_name}_yearly_trends.json'), orient='records', indent=2 ) plt.savefig( os.path.join(args.output, f'{drug_name}_yearly_trends.png'), dpi=150, bbox_inches='tight' ) ``` It is also used in the recent-events and unconditional summary writes: ```python events_path = os.path.join(args.output, f'{drug_name}_recent_events.json') with open(events_path, 'w') as f: json.dump(events_data, f, indent=2) # Summary JSON summary_path = os.path.join(args.output, f'{drug_name}_summary.json') with open(summary_path, 'w') as f: json.dump(results, f, indent=2) ``` ### Technical Analysis `os.path.join()` does not enforce containment within its first argument. If the filename component is absolute, the preceding output directory is discarded. Relative values containing directory separators a ...[truncated 2554 chars]
- Remediation
- ## Remediation Suggestions 1. Never use the raw drug name as a filesystem component. Create a dedicated filename slug that permits only a narrow set of characters, such as ASCII letters, digits, underscores, and hyphens. 2. Replace all separators, traversal components, control characters, and leading dots. Use a stable hash when the sanitized value would be empty or ambiguous. 3. Apply the same sanitization to names returned by PubChem because remote API data must not be trusted as a safe filename. 4. Resolve and validate every destination before writing: ```python from pathlib import Path import hashlib import re def safe_label(value): label = re.sub(r'[^A-Za-z0-9_-]+', '_', value).strip('._-') if not label: label = hashlib.sha256(value.encode()).hexdigest()[:16] return label[:100] output_root = Path(args.output).resolve() output_root.mkdir(parents=True, exist_ok=True) label = safe_label(drug_name) def output_path(suffix): destination = (output_root / f"{label}{suffix}").resolve() if output_root not in destination.parents: raise ValueError("Output path escapes the configured directory") return destination ``` 5. Use `output_path("_summary.json")` and equivalent validated destinations for every JSON and PNG write. 6. Consider exclusive file creation or an explicit overwrite option when existing files must not be replaced silently. 7. Run the Skill under a least-privileged account with write access limited to a dedicated output directory. 8. Add regression tests covering absolute paths, `../` traversal, nested separators, leading dots, Unicode separators, empty labels, and hostile names returned by PubChem.