T09 · Insecure Skill Coding Practices
Error
- Location
scripts/sync_db.py:20- Finding
Path Traversal Allows Arbitrary Markdown File and Directory Writes
- Content
View full analysis
dict: """Ensure all subdirectories exist for a member, return base path.""" base = os.path.join(RECORDS_DIR, member_code) for subdir in ["visits", "medications", "exams", "daily_vitals", "attachments"]: os.makedirs(os.path.join(base, subdir), exist_ok=True) os.makedirs(os.path.join(base, "attachments", "2026"), exist_ok=True) os.makedirs(os.path.join(base, "attachments", "2027"), exist_ok=True) return base ``` ```python for row in members: r = dict(zip(cols, row)) base = ensure_dirs(r["member_code"]) profile_path = os.path.join(base, "profile.md") ``` ```python base = ensure_dirs(member_code) visits_dir = os.path.join(base, "visits") for row in visits: v = dict(zip(cols, row)) visit_path = os.path.join(visits_dir, f"{v['visit_id']}.md") # Content generation omitted here; the destination is opened in # destructive write mode. with open(visit_path, "w", encoding="utf-8") as f: f.write(content) ``` ```python base = ensure_dirs(member_code) meds_dir = os.path.join(base, "medications") for row in meds: m = dict(zip(cols, row)) med_path = os.path.join(meds_dir, f"{m['medication_id']}.md") # Content generation omitted here; the destination is opened in # destructive write mode. with open(med_path, "w", encoding="utf-8") as f: f.write(content) ``` ### Technical Analysis The exporter treats database-controlled `member_code`, `visit_id`, and `medication_id` values as trusted filesystem path components. It does not reject absolute paths, path separators, or parent-directory components such as `..`. Python's `os.path.join()` does not enforc ...[truncated 2367 chars]- Remediation
View remediation
str: if not isinstance(value, str) or not SAFE_ID.fullmatch(value): raise ValueError(f"Unsafe {field}") return value ``` 2. Canonicalize every destination and verify that it remains under the intended export root: ```python from pathlib import Path EXPORT_ROOT = Path(RECORDS_DIR).resolve() def contained_path(*parts: str) -> Path: destination = EXPORT_ROOT.joinpath(*parts).resolve() if destination != EXPORT_ROOT and EXPORT_ROOT not in destination.parents: raise ValueError("Export path escapes the records directory") return destination ``` 3. Validate `member_code`, `visit_id`, and `medication_id` independently. Reject absolute paths, `.` or `..` components, path separators, null bytes, and platform-specific alternate separators. 4. Add equivalent constraints at the data-entry boundary and, where practical, SQLite `CHECK` constraints so unsafe identifiers cannot enter the database. 5. Avoid silently overwriting existing files. Use exclusive creation (`"x"`) by default, or require an explicit overwrite option. For legitimate replacement, write to a safe temporary file in the same directory and atomically replace the destination. 6. Run the exporter as an unprivileged account with write access limited to the intended export directory. 7. Add regression tests covering absolute paths, nested traversal, mixed separators, symbolic links, and crafted identifiers imported from legacy records. ]]>