T09 · Insecure Skill Coding Practices
- Location
watermark_remover.py:135- Finding
Unvalidated Task Subdirectory Allows Writes Outside the Configured Input Directory
- Content
View full analysis
Vulnerability Details
File Location:
watermark_remover.py, lines 135–137
Vulnerability Type: Path traversal / unrestricted filesystem write
Risk Level: HighVulnerable Code
python if task_subdir is None: target_root = self.input_dir sub = "" else: sub = task_subdir target_root = self.input_dir / sub target_root.mkdir(parents=True, exist_ok=True) # First write under a temporary .part name, then rename it. delivered: list[Path] = [] for src in srcs: final_path = target_root / src.name tmp_path = final_path.with_suffix(final_path.suffix + ".part") try: if copy: shutil.copy2(src, tmp_path) else: shutil.move(str(src), str(tmp_path)) tmp_path.rename(final_path)Technical Analysis
The
task_subdirvalue can be supplied through the command-line--subdiroption and is used directly in a filesystem path without validation. Python path joining does not guarantee that the result remains belowself.input_dir.A value containing parent-directory components, such as
../../target, can traverse outside the configured input directory. An absolute path can replace the intended base path entirely on supported platforms. The script then creates the attacker-selected directory and copies or moves source images into it.Because no canonical containment check is performed, the intended security boundary around the GUI watch directory is not enforced. The final filename is limited to the source filename, but the destination directory is attacker-controlled.
Attack Path
- An attacker causes the Agent or user to process an image while supplying a crafted task subdirectory, for example:
bash python watermark_remover.py process image.png --subdir ../../unintended-directory target_rootbecomes a path outside the configuredinput_dir.- `mkdir(parents=True, exist_ok=Tru ...[truncated 1202 chars]
- An attacker causes the Agent or user to process an image while supplying a crafted task subdirectory, for example:
- Remediation
View remediation
Remediation Suggestions
Treat
task_subdiras a single directory identifier rather than an arbitrary path.- Reject absolute paths.
- Reject
.and..path components. - Reject directory separators and platform-specific drive or UNC path syntax if nested task paths are unnecessary.
- Resolve the destination and verify that it remains beneath the resolved input directory before creating directories or writing files.
- Consider generating task directory names internally and disallowing user-defined paths entirely.
- Add tests covering parent traversal, absolute POSIX paths, Windows drive paths, UNC paths, and symbolic-link escapes.
Example hardening:
python def safe_task_root(input_dir: Path, task_subdir: str) -> Path: candidate_name = Path(task_subdir) if ( candidate_name.is_absolute() or len(candidate_name.parts) != 1 or task_subdir in {"", ".", ".."} or candidate_name.name != task_subdir ): raise ValueError("Invalid task subdirectory name") base = input_dir.resolve() candidate = (base / candidate_name).resolve() try: candidate.relative_to(base) except ValueError as exc: raise ValueError("Task directory escapes input_dir") from exc return candidateIf directories inside
input_dirmay contain attacker-controlled symbolic links, perform filesystem operations using mechanisms that prevent symbolic-link traversal, or ensure the selected directory is newly created and verified before use.