T09 · Insecure Skill Coding Practices
Error
- Location
scripts/restore_state.py:214- Finding
Unsafe Archive Extraction and Restore Path Traversal
- Content
View full analysis
Path: rel_path = Path(rel) parts = rel_path.parts if len(parts) < 3: raise RuntimeError(f"Unexpected archive path: {rel}") scope = parts[1] remainder = Path(*parts[2:]) if scope == "workspace": return workspace / remainder if scope == "state": return state_dir / remainder raise RuntimeError(f"Unknown archive scope: {scope}") ``` ```python with tempfile.TemporaryDirectory(prefix="openclaw-restore-") as td: tmpdir = Path(td) with tarfile.open(archive, "r:gz") as tar: tar.extractall(tmpdir) ``` ```python rollback_archive = make_rollback(workspace, state_dir, rollback_dir, include_prefixes, exclude_prefixes) restored_paths = [] for item in verified_files: src = tmpdir / item["path"] dst = archive_to_dest(item["path"], workspace, state_dir) dst.parent.mkdir(parents=True, exist_ok=True) shutil.copy2(src, dst) restored_paths.append(str(dst)) ``` ### Technical Analysis The restore operation passes every archive member to `tar.extractall()` before validating the archive manifest, member types, or member paths. On Python versions or configurations that do not enforce a safe extraction filter, malicious absolute paths, parent-directory components, symbolic links, hard links, or special archive entries can cause extraction outside the temporary directory. There is a second, independent path-containment issue in `archive_to_dest()`. The function checks the archive scope but does not reject `..` components, absolute remainders, or resolved paths outside the selected workspace or state directory. For example, ...[truncated 2091 chars]- Remediation
View remediation
Path: root = root.resolve() if relative.is_absolute() or ".." in relative.parts: raise RuntimeError(f"Unsafe relative path: {relative}") candidate = (root / relative).resolve() if not candidate.is_relative_to(root): raise RuntimeError(f"Path escapes allowed root: {relative}") return candidate ``` 4. **Validate before any filesystem write** - Load the manifest without extracting arbitrary archive content. - Validate its schema, path prefixes, unique entries, checksums, and member correspondence. - Only after successful structural validation should regular files be written to the temporary directory. 5. **Add adversarial tests** - Test absolute member names and `../` traversal. - Test traversal in manifest paths. - Test symbolic-link and hard-link entries. - Test duplicate ...[truncated 121 chars]