T09 · Insecure Skill Coding Practices
Warning
- Location
scripts/migrate-v1-to-v2.sh:48- Finding
Path Traversal Through Unvalidated Member Identifiers
- Content
View full analysis
None: nonlocal current if not current: return member_id = current.get("id", "") name = current.get("name", "") alias = current.get("alias", "") if not member_id or not name: errors.append(f"成员配置缺少 id 或 name: {current}") else: members.append(Member(member_id, name, alias)) for key in [name, alias]: if not key: continue old = identity_owner.get(key) if old and old != member_id: errors.append(f"成员名称/别名冲突: {key} 同时指向 {old} 和 {member_id}") identity_owner[key] = member_id current = None ``` The unchecked value is subsequently used to construct and write the migration destination: ```python for member in members: for suffix, target_name in mapping.items(): src = members_dir / f"{member.name}-{suffix}.md" if not src.exists(): continue dst = members_dir / member.member_id / target_name planned.append((src, dst, member)) ``` ```python for src, dst, _ in planned: backup_target = backup_dir / src.relative_to(data_dir) backup_target.parent.mkdir(parents=True, exist_ok=True) shutil.copy2(src, backup_target) dst.parent.mkdir(parents=True, exist_ok=True) shutil.copy2(src, dst) ``` The importer contains the same unchecked configuration parsing pattern: ```python def finish_current() -> None: nonlocal current if not curren ...[truncated 4784 chars]- Remediation
View remediation
str: if not MEMBER_ID_RE.fullmatch(member_id): raise ValueError(f"Invalid member ID: {member_id!r}") return member_id ``` 2. Resolve every destination and explicitly enforce containment: ```python members_root = (data_dir / "members").resolve() destination = (members_root / member_id / "timeline.md").resolve() if not destination.is_relative_to(members_root): raise TeamMemoryPathError("Member path escapes the locked members directory") ``` 3. Apply containment checks before all reads, writes, copies, backups, directory creation, and existence checks. 4. Reject absolute identifiers, path separators, `.` and `..` components, control characters, and platform-specific separator variants. 5. Reject symlinked destination directories and files. Where practical, inspect each existing path component with `lstat()` and fail if any component is a symbolic link. 6. Validate member IDs parsed from both `skill-config.yaml` and `tasks/tasks.md`; do not assume that Markdown generated by another script remains trusted. 7. Add regression tests covering relative traversal, absolute paths, nested separators, symlink destinations, and valid `member-NNN` identifiers. ]]>