T09 · Insecure Skill Coding Practices
Warning
- Location
- converter.py:82
- Finding
- Unvalidated Workflow Name Allows Path Traversal and Arbitrary File Overwrite## Vulnerability Details **File Location**: `converter.py:82-84`, with the affected argument defined at `converter.py:570` **Vulnerability Type**: Path traversal and arbitrary file write **Risk Level**: Medium ### Vulnerable Code ```python output_file = os.path.join(yaml_dir, app_name + ".yaml") node_list = [] ``` The resulting path is subsequently opened for writing at `converter.py:283-284`: ```python with open(output_file, 'w', encoding='utf-8') as yaml_file: yaml.dump(general_template, yaml_file, allow_unicode=True, default_flow_style=False) ``` The workflow name is accepted without validation at `converter.py:570`: ```python parser.add_argument('--name', type=str, required=True, help='Workflow name') ``` The shell wrapper also forwards this value unchanged at `bash_converter.sh:24-34`: ```bash NAME="${2:-workflow}" OUTPUT_PATH="${3:-${DEFAULT_OUTPUT_PATH}}" TYPE="${4:-dify}" python "${SCRIPT_DIR}/converter.py" \ --json_path "${JSON_PATH}" \ --name "${NAME}" \ --output_path "${OUTPUT_PATH}" \ --type "${TYPE}" ``` ### Technical Analysis `resolve_safe_output_path()` validates only the user-supplied output directory. It does not validate the workflow name used to construct the final filename. Because `app_name` can contain absolute paths, `..` components, or path separators, `os.path.join(yaml_dir, app_name + ".yaml")` does not guarantee that the result remains beneath `yaml_dir`. An absolute `app_name` causes Python to discard the preceding directory. A relative name containing traversal components can resolve outside the approved output directory. The Dify conversion path then opens the constructed path using write mode. If the destination exists and is writable, it is truncated and replaced with generated YAML. If the necessary parent directories already exist, a new file can also be created outside the configured output directory. ...[truncated 2059 chars]
- Remediation
- ## Remediation Suggestions 1. Restrict workflow names to a conservative basename format: ```python import re WORKFLOW_NAME_RE = re.compile(r"^[A-Za-z0-9_-]+$") def validate_workflow_name(name: str) -> str: if not WORKFLOW_NAME_RE.fullmatch(name): raise ValueError( "Workflow name may contain only ASCII letters, digits, " "underscores, and hyphens." ) return name ``` 2. Explicitly reject absolute paths, `.` and `..`, path separators, NUL characters, and platform-specific alternate separators. 3. Validate every complete destination after combining its directory and filename: ```python def safe_child_path(parent: str, filename: str) -> str: parent_real = os.path.realpath(parent) candidate = os.path.realpath(os.path.join(parent_real, filename)) if os.path.commonpath([parent_real, candidate]) != parent_real: raise ValueError("Output path escapes its approved directory") return candidate ``` 4. Apply the containment check independently to: - Dify YAML output. - Coze temporary YAML output. - Coze workflow staging directories. - Final Coze ZIP output. 5. Avoid silently rewriting unsafe input. Reject it with a clear error so callers cannot mistakenly believe their requested path was used. 6. Add regression tests covering absolute paths, `../`, nested traversal, Windows separators, drive-qualified paths, UNC paths, symlinked directories, and ordinary valid names. 7. Correct `CONVERTER_USAGE.md` so its output-containment claims match the behavior actually enforced by the implementation.