T09 · Insecure Skill Coding Practices
- Location
scripts/import_package.py:223- Finding
Untrusted Migration Package Allows Arbitrary File Writes Through Path Traversal
- Content
View full analysis
Vulnerability Details
File Location:
scripts/import_package.py, lines 223–319
Vulnerability Type: ZIP archive path traversal leading to arbitrary file writes
Risk Level: HighTechnical Analysis
The documented import workflow accepts migration ZIP files that may originate from another workspace, device, or package provider. Both the ZIP member names and the package's
PROVENANCE.jsonare therefore package-controlled inputs.The default namespace is derived from the untrusted
source_nodefield without restricting path separators or traversal components:python with zipfile.ZipFile(args.package, "r") as z: raw = z.read("PROVENANCE.json").decode("utf-8", "ignore") prov = json.loads(raw) source_node = prov.get("source_node", "") first_code = source_node.split(",")[0].strip() or "R0" # ... prefix = args.namespace or ("B-" + first_code) # ... ns_dir = os.path.join(ws, prefix) os.makedirs(ns_dir, exist_ok=True)More critically,
landed_of()validates absolute paths and..components only in its fallback branch. Entries beginning withhandoffs/orartifacts/are joined directly to destination directories without containment validation:python def landed_of(rel: str): rel = rel.replace("\\", "/") if rel == "INDEX.excerpt.md": return os.path.join(ns_dir, "INDEX.excerpt.md") if rel.startswith("handoffs/"): base = rel[len("handoffs/"):] return os.path.join(ho_dir, f"{prefix}_{base}") if rel.startswith("artifacts/"): rest = rel[len("artifacts/"):] return os.path.join(ns_dir, "artifacts", rest) # Other files, such as top-level CONTEXT_DIGEST.md, are placed at # the namespace root. # Safety guard: reject absolute paths and .. traversal if os.path.isabs(rel) or ".." in rel.split("/"): return None return os.path.join(ns_dir, rel)Consequently, a member such as:
text artifacts/../../../../.config/application/config.pyproduces a d ...[truncated 4128 chars]
- Remediation
View remediation
Remediation Suggestions
-
Validate every archive member before extraction
- Convert separators to a canonical form.
- Reject absolute, drive-qualified, UNC, empty, and NUL-containing paths.
- Reject any
.or..path component. - Apply these checks uniformly to
handoffs/,artifacts/, and all fallback entries.
-
Enforce destination containment
- Resolve both the extraction root and candidate destination with
realpath. - Use
os.path.commonpath()to ensure the resolved destination remains under its designated root. - Perform containment checks after joining paths, not only against the original member name.
- Use separate roots for artifacts, handoffs, and namespace-level files.
- Resolve both the extraction root and candidate destination with
-
Constrain namespace values
- Validate
--namespaceandPROVENANCE.source_nodeagainst a strict allowlist, such as letters, digits,_, and-. - Reject path separators, drive delimiters, control characters,
.components, and traversal sequences. - Do not use package metadata directly as a filesystem path component without validation.
- Validate
-
Validate before writing
- Read and validate the entire member list and manifest before creating any destination files.
- Require every non-directory content member to appear exactly once in
PROVENANCE.files. - Reject undeclared members and duplicate or normalized-name collisions.
- Verify member hashes in a private temporary directory first.
-
Use transactional extraction
- Extract into a securely created temporary directory.
- Complete path validation and hash verification there.
- Move the validated tree into the workspace atomically.
- On any failure, remove the temporary tree and leave the destination workspace unchanged.
-
Fail closed
- Abort immediately on a missing file, hash mismatch, unsafe path, duplicate member, malformed provenance record, or unsupported entry type.
- Do not update provenance records or
INDEX.mdafter failed ver ...[truncated 386 chars]
-