T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/obsidian_sync.py:440
- Finding
- Arbitrary File Read Through Obsidian Import Path Traversal<![CDATA[ ## Vulnerability Details **File Location**: `scripts/obsidian_sync.py:440-460` **Vulnerability Type**: Path traversal leading to arbitrary local file read **Risk Level**: High ### Vulnerable Code ```python file_rel = data.get("file", "") if not file_rel: output_error("笔记文件路径（file）为必填字段", code="VALIDATION_ERROR") return vault_path = data.get("vault_path") or _get_vault_path() if not vault_path: state = _get_sync_state() vault_path = state.get("vault_path") if not vault_path or not os.path.isdir(vault_path): output_error("未连接到 Obsidian 笔记库，请先执行 connect 操作", code="NOT_CONNECTED") return fpath = os.path.join(vault_path, file_rel) if not os.path.exists(fpath): output_error(f"笔记文件不存在: {file_rel}", code="FILE_NOT_FOUND") return try: with open(fpath, "r", encoding="utf-8") as f: raw_content = f.read() ``` ### Technical Analysis The `import_draft` operation describes `file` as a path relative to the configured Obsidian vault, but the implementation does not enforce that restriction. `os.path.join(vault_path, file_rel)` does not provide path containment. A value containing `../` components can resolve outside the vault. If `file_rel` is an absolute path, `os.path.join` discards `vault_path` entirely. The implementation also does not resolve symlinks, so a symlink located inside the vault can point to a file outside it. The subsequent existence check only establishes that the resulting path exists. It does not verify that the resolved path remains under the configured vault root. ### Attack Path 1. The attacker or untrusted caller invokes the `import-draft` action. 2. The supplied JSON sets `file` to a traversal or absolute path, such as: ```json { "file": "../../sensitive-file.md" } ``` 3. The code joins the crafted value with the vault path without normalization or containment validation. 4. `open()` reads the external file using the privileges of the Agent process. 5. The file contents ...[truncated 652 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Create a shared helper that resolves and validates all vault paths before any filesystem access: ```python def resolve_vault_path(vault_path: str, relative_path: str) -> str: if not relative_path or os.path.isabs(relative_path): raise ValueError("The file path must be relative to the vault") vault = os.path.realpath(os.path.expanduser(vault_path)) target = os.path.realpath(os.path.join(vault, relative_path)) try: inside_vault = os.path.commonpath([vault, target]) == vault except ValueError: inside_vault = False if not inside_vault: raise ValueError("The file path must remain inside the vault") return target ``` Use this helper instead of directly calling `os.path.join`. In addition: 1. Reject absolute paths and traversal outside the vault. 2. Require the imported target to be a regular `.md` file. 3. Resolve symlinks before checking containment. 4. Consider rejecting symlink targets entirely when imports are expected to operate only on ordinary vault files. 5. Avoid returning unnecessary external path information in error messages. 6. Add tests for absolute paths, `../` traversal, nested traversal, symlink escapes, and valid nested vault notes. ]]>