T09 · Insecure Skill Coding Practices
- Location
scripts/query_faers.py:73- Finding
User-Controlled Path Traversal and File Overwrite in FAERS Output Generation
- Content
View full analysis
Vulnerability Details
File Location:
scripts/query_faers.py, lines 73–156
Vulnerability Type: Path traversal and arbitrary file overwrite
Risk Level: HighThe user-controlled
--drugargument 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
View remediation
Remediation Suggestions
-
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.
-
Replace all separators, traversal components, control characters, and leading dots. Use a stable hash when the sanitized value would be empty or ambiguous.
-
Apply the same sanitization to names returned by PubChem because remote API data must not be trusted as a safe filename.
-
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 -
Use
output_path("_summary.json")and equivalent validated destinations for every JSON and PNG write. -
Consider exclusive file creation or an explicit overwrite option when existing files must not be replaced silently.
-
Run the Skill under a least-privileged account with write access limited to a dedicated output directory.
-
Add regression tests covering absolute paths,
../traversal, nested separators, leading dots, Unicode separators, empty labels, and hostile names returned by PubChem.
-