T09 · Insecure Skill Coding Practices
Error
- Location
scripts/common.py:44- Finding
Path Traversal Through Unvalidated Pet Identifiers
- Content
View full analysis
Dict[str, Any]: path = storage_root() / 'pets' / f'{pet_id}.json' data = read_json(path) if not data: raise SystemExit(f'Pet not found: {pet_id}') return data ``` ```python # scripts/pet_manager.py:9-10 pet_id = args.pet_id or slugify(args.name) path = storage_root() / 'pets' / f'{pet_id}.json' ``` ```python # scripts/pet_manager.py:36-37 path = storage_root() / 'pets' / f'{args.pet_id}.json' data = read_json(path) ``` ```python # scripts/pet_manager.py:56-57 path = storage_root() / 'pets' / f'{args.pet_id}.json' data = read_json(path) ``` ```python # scripts/reminder_manage.py:9-10 load_pet(args.pet_id) path = storage_root() / 'reminders' / f'{args.pet_id}.json' ``` ```python # scripts/reminder_manage.py:27-28 path = storage_root() / 'reminders' / f'{args.pet_id}.json' data = read_json(path, {'pet_id': args.pet_id, 'reminders': []}) ``` ```python # scripts/reminder_check.py:11-13 def load_reminders(pet_id=None): root = storage_root() / 'reminders' files = [root / f'{pet_id}.json'] if pet_id else sorted(root.glob('*.json')) ``` ### Technical Analysis User-supplied `pet_id` values are interpolated directly into filesystem paths. Only automatically generated identifiers pass through `slugify`; an explicit `--pet-id` is accepted without validation. Python's `pathlib` path composition does not enforce containment. A value containing `../` can escape the intended `pets` or `reminders` directory. If the final component is absolute, it replaces the preceding storage-root components entirely. The `.json` suffix limits targets to JSON filenames but does not ...[truncated 1978 chars]- Remediation
View remediation
str: if not PET_ID_PATTERN.fullmatch(pet_id): raise SystemExit('Invalid pet ID') return pet_id ``` 2. Reject absolute paths, directory separators, empty identifiers, `.` components, and `..` components rather than attempting to normalize them silently. 3. Resolve every constructed path and enforce containment: ```python def contained_json_path(directory: Path, pet_id: str) -> Path: pet_id = validate_pet_id(pet_id) base = directory.resolve() candidate = (base / f'{pet_id}.json').resolve() try: candidate.relative_to(base) except ValueError: raise SystemExit('Path escapes the storage directory') return candidate ``` 4. Apply the helper consistently in: - `load_pet` - Pet creation, update, and view operations - Reminder creation and listing - Reminder checking by pet ID 5. Do not rely solely on `slugify`, because it transforms invalid identifiers and may create collisions. Explicit identifiers should be validated and rejected if malformed. 6. Add regression tests covering: - `../` traversal - Absolute paths - Embedded `/` and `\` - Empty and dot-only identifiers - Valid identifiers - Verification that no file outside the designated directory is read or modified ]]>