T09 · Insecure Skill Coding Practices
Warning
- Location
- fa_advisor/advisor.py:128
- Finding
- Path Traversal Allows PDF Writes Outside the Intended Output Directory<![CDATA[ ## Vulnerability Details **File Location**: `fa_advisor/advisor.py:128-145`, `fa_advisor/advisor.py:188-198`, and `fa_advisor/types/project.py:181` **Vulnerability Type**: Unsanitized user-controlled path construction **Risk Level**: Medium ### Vulnerable Code The startup package constructs report paths directly from the user-controlled project name: ```python # 6. Generate PDF reports if requested if generate_pdf: print("📝 Generating PDF reports...") output_dir = Path("output") output_dir.mkdir(exist_ok=True) result['pdf_reports'] = { 'assessment': await self.pdf_generator.generate_assessment_report( assessment, project.name, output_dir / f"{project.name}_assessment.pdf" ), 'valuation': await self.pdf_generator.generate_valuation_report( valuation, project.name, output_dir / f"{project.name}_valuation.pdf" ) } print("✅ PDF reports generated in 'output' directory\n") ``` The investor package uses the same unsafe filename construction: ```python # Generate PDF memo if requested if generate_pdf: print("📝 Generating PDF investment memo...") output_dir = Path("output") output_dir.mkdir(exist_ok=True) result['pdf_memo'] = await self.pdf_generator.generate_investment_memo( memo, project.name, output_dir / f"{project.name}_investment_memo.pdf" ) print("✅ PDF memo generated\n") ``` The project model only verifies that the name is non-empty: ```python class Project(BaseModel): """ Complete project/startup information This is the main data structure for a startup project seeking funding or being evaluated by investors. """ # Basic Information name: str = Field(min_length=1) ``` The PDF generator then writes directly to the supplied path: ```python output_path = Path(output_path) doc = SimpleDocTemplate( str(output_path), pagesize=self.pag ...[truncated 2794 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Do not derive destination paths directly from company names.** Prefer a server-generated identifier such as a UUID: ```python from uuid import uuid4 report_name = f"{uuid4().hex}_assessment.pdf" ``` 2. **If readable filenames are required, apply a strict allowlist.** Replace every character other than letters, digits, hyphens, and underscores: ```python import re def safe_filename(value: str) -> str: value = re.sub(r"[^A-Za-z0-9_-]+", "_", value).strip("_") if not value: raise ValueError("Project name cannot produce an empty filename") return value[:100] ``` 3. **Resolve and enforce directory containment before writing:** ```python output_dir = Path("output").resolve() output_dir.mkdir(parents=True, exist_ok=True) safe_name = safe_filename(project.name) destination = (output_dir / f"{safe_name}_assessment.pdf").resolve() if output_dir not in destination.parents: raise ValueError("Invalid report destination") ``` 4. **Reject path semantics explicitly.** Company names used in filenames should not contain `/`, `\`, `..`, null bytes, drive prefixes, or absolute-path components. 5. **Avoid unintended overwrites.** Create reports using exclusive file creation or reject destinations that already exist unless an authorized caller explicitly enables replacement. 6. **Apply the same centralized helper to every generated report**, including assessment, valuation, and investment-memo output paths. 7. **Add regression tests** for names containing: - `../` - `..\` - Absolute POSIX paths - Windows drive paths - Repeated separators - Empty names after sanitization - Existing destination filenames ]]>