T09 · Insecure Skill Coding Practices
- Location
scripts/mesh_tools.py:31- Finding
Unrestricted Extraction of User-Controlled 3MF Archives
- Content
View full analysis
Path: """Extract a .3mf (zip) to dest_dir/extracted and return that path.""" dest_dir = Path(dest_dir) out = dest_dir / "extracted" if out.exists(): shutil.rmtree(out) out.mkdir(parents=True) with zipfile.ZipFile(archive_path) as z: z.extractall(out) return out ``` ### Technical Analysis A 3MF file is a ZIP archive and may be supplied by an untrusted user. The function passes every archive member directly to `ZipFile.extractall()` without independently validating: - Absolute or traversal-oriented member paths - Whether the resolved extraction target remains under `out` - Archive member types and platform-specific path semantics - The number of archive entries - Individual or total uncompressed sizes - Compression ratios - Duplicate or conflicting member names The absence of explicit containment checks creates a path-traversal risk on runtimes or archive path variants for which extraction sanitization is incomplete or bypassable. Even where the Python runtime normalizes common `../` entries, the function still relies implicitly on runtime-specific behavior rather than enforcing its own security boundary. The lack of resource limits also permits ZIP-bomb-style denial of service. A small archive can declare highly compressed entries that expand to a very large size, exhausting disk space, memory, or processing time. The function additionally removes an existing `dest_dir/extracted` tree recursively. Although the path is deterministically appended with `extracted`, callers must still ensure that `dest_dir` is trusted and that the resulting path cannot be redirected through unsafe filesystem objects. ### Attack Path 1. An attacker prov ...[truncated 1283 chars]- Remediation
View remediation
Path: root = root.resolve() member = Path(member_name) if member.is_absolute() or ".." in member.parts: raise ValueError(f"unsafe archive member: {member_name}") destination = (root / member).resolve() if destination != root and root not in destination.parents: raise ValueError(f"archive member escapes extraction root: {member_name}") return destination ``` All entries should be validated before any entry is written. Declared size checks should also be supplemented with streaming byte limits because archive metadata may be deceptive. ]]>
