Back to skill

Security audit

File Sync

Security checks for vulnerabilities and agentic risk

Overview

This file-sync skill does what it claims, but its bundled sync script can be tricked into reading, copying, or moving files outside the chosen folders.

Review this skill carefully before installing. It is not trying to hide what it does, but it should only be used on folders you fully trust and control; avoid syncing attacker-controlled USB drives or shared folders until path validation and symlink rejection are added.

Vulnerability Patterns
  • Unauthorized Access and Privilege EscalationObtains permissions beyond the task's legitimate needs
  • Insecure Skill Coding PracticesFinds exploitable flaws such as hardcoded secrets or command injection
  • Skill Instruction HijackingAlters the agent's session goals or safety constraints when the skill loads
  • Agent Memory PoisoningWrites attacker-controlled rules into memory that affect later sessions
  • Remote Payload Retrieval and ExecutionFetches external code whose behavior can change after review
Findings (3)

T05 · Unauthorized Access and Privilege Escalation

Error
Location
scripts/sync.py:181
Finding
Unvalidated State Paths Permit Filesystem Access Outside Synchronization Roots<![CDATA[ ## Vulnerability Details **File Location**: `scripts/sync.py:124-132, 181-188, 269-278` **Vulnerability Type**: Path traversal through untrusted synchronization state **Risk Level**: High ### Vulnerable Code ```python def move_to_trash(root, rel_path, log_file): src = os.path.join(root, rel_path) if not os.path.exists(src): return dst = os.path.join(root, ".trash", rel_path + f".{int(time.time())}") os.makedirs(os.path.dirname(dst), exist_ok=True) shutil.move(src, dst) log(log_file, "TRASH", rel_path) ``` ```python all_files = set(local_now) | set(remote_now) | set(local_old) | set(remote_old) for f in all_files: try: l = local_now.get(f) r = remote_now.get(f) lo = local_old.get(f) ro = remote_old.get(f) local_path = os.path.join(local_root, f) remote_path = os.path.join(remote_root, f) ``` ```python if l and not r: os.makedirs(os.path.dirname(remote_path), exist_ok=True) shutil.copy2(local_path, remote_path) remote_now[f] = l.copy() log(log_file, "INFO", f"COPY NEW → remote {f}") stats["copy"] += 1 continue if r and not l: os.makedirs(os.path.dirname(local_path), exist_ok=True) shutil.copy2(remote_path, local_path) ``` ### Technical Analysis The keys loaded from `.sync_state.json` are used as filesystem paths without validation. `load_state()` accepts the persisted `files` object, and its keys are subsequently added to `all_files`, even when those keys were not generated by the current filesystem scan. A key can contain an absolute path or parent-directory traversal components such as `../../target`. When the key is passed to `os.path.join`, an absolute final component causes Python to discard the preceding root. Traversal components can similarly resolve outside the intended synchronization directory. These paths reach sensitive filesystem operations including `shutil.move`, `shutil.copy2`, and `os.makedirs`. No canonicali ...[truncated 1305 chars]
Remediation
<![CDATA[ ## Remediation Suggestions - Treat all `.sync_state.json` keys as untrusted input. - Reject absolute paths, empty paths, NUL characters, and any path containing a `..` component. - Resolve each candidate path before use and verify containment with `os.path.commonpath`. ```python def safe_path(root, rel_path): if not isinstance(rel_path, str) or not rel_path: raise ValueError("Invalid relative path") if os.path.isabs(rel_path): raise ValueError("Absolute paths are not allowed") root_real = os.path.realpath(root) candidate = os.path.realpath(os.path.join(root_real, rel_path)) if os.path.commonpath([root_real, candidate]) != root_real: raise ValueError("Path escapes synchronization root") return candidate ``` - Apply the containment check separately for every source and destination root before calling `open`, `os.stat`, `os.makedirs`, `shutil.copy2`, or `shutil.move`. - Prefer deriving path keys exclusively from the current live scan. Persisted state should be treated as metadata associated with already validated paths. - Validate the complete state schema, including key and value types, before processing it. - If an invalid state entry is encountered, stop safely and report the error rather than silently processing or discarding it. ]]>

T05 · Unauthorized Access and Privilege Escalation

Error
Location
scripts/sync.py:78
Finding
Symbolic-Link Following Permits Reads and Writes Outside Synchronization Roots<![CDATA[ ## Vulnerability Details **File Location**: `scripts/sync.py:78-110, 223-241, 269-278` **Vulnerability Type**: Improper symbolic-link handling and filesystem boundary bypass **Risk Level**: High ### Vulnerable Code ```python def scan_folder(root, old_state): state = {} for dirpath, dirnames, filenames in os.walk(root): dirnames[:] = [d for d in dirnames if d not in IGNORE_DIRS] for f in filenames: if f == STATE_FILE: continue full = os.path.join(dirpath, f) rel = os.path.relpath(full, root) stat = os.stat(full) size = stat.st_size mtime = int(stat.st_mtime) old = old_state.get(rel) # ===== Determine whether the hash must be recalculated ===== if old and old["size"] == size and old["mtime"] == mtime: state[rel] = old continue h = file_hash(full) history = old.get("history", [])[:] if old else [] if not history or history[-1] != h: history.append(h) history = history[-HISTORY_LIMIT:] state[rel] = { "size": size, "mtime": mtime, "hash": h, "history": history, "deleted": False, } ``` ```python if lh == base: os.makedirs(os.path.dirname(local_path), exist_ok=True) shutil.copy2(remote_path, local_path) local_now[f]["history"] = merge_history(remote_now, local_now, f) log(log_file, "INFO", f"COPY remote→local {f}") stats["copy"] += 1 continue if rh == base: os.makedirs(os.path.dirname(remote_path), exist_ok=True) shutil.copy2(local_path, remote_path) remote_now[f]["history"] = merge_history(local_now, remote_now, f) log(log_file, "INFO", f"COPY local→remote {f}") stats["copy"] += 1 continue ``` ```python if l and not r: os.makedirs(os.path.dirname ...[truncated 2585 chars]
Remediation
<![CDATA[ ## Remediation Suggestions - Reject symbolic-link files and directories during scanning by using `os.lstat()` and checking `stat.S_ISLNK()`. - Inspect every component of source and destination paths rather than checking only the final component. - Resolve paths and confirm that they remain beneath their expected roots before every read, copy, move, or write operation. - Define and document explicit symlink behavior. The safest default is to skip symlinks and log a warning. - Where the operating system supports them, use descriptor-relative filesystem operations with no-follow protections such as `O_NOFOLLOW`, `dir_fd`, and directory file descriptors. - Revalidate immediately before mutation to reduce time-of-check/time-of-use exposure. - Do not permit synchronization roots themselves to be symlinks unless they are resolved once at startup and the resolved roots are used consistently. - Add tests covering file symlinks, directory symlinks, broken links, chained links, and concurrent link replacement. ]]>

T09 · Insecure Skill Coding Practices

Warning
Location
scripts/sync.py:62
Finding
Collision-Broken MD5 Is Used for File Integrity and Conflict Decisions<![CDATA[ ## Vulnerability Details **File Location**: `scripts/sync.py:62-66, 203-214` **Vulnerability Type**: Use of a cryptographically broken hash algorithm for integrity-sensitive decisions **Risk Level**: Medium ### Vulnerable Code ```python def file_hash(path): h = hashlib.md5() with open(path, "rb") as f: while chunk := f.read(8192): h.update(chunk) return h.hexdigest() ``` ```python if l and r: lh = l["hash"] rh = r["hash"] if lh == rh: continue base = find_common_ancestor(l["history"], r["history"]) if base is None or (lh != base and rh != base): handle_conflict(local_root, remote_root, f, device_name, log_file) stats["conflict"] += 1 continue ``` ### Technical Analysis MD5 is collision-broken and is unsuitable for adversarial integrity comparisons. The synchronization logic treats matching MD5 values as proof that two files have identical content and also stores MD5 values as version-history identifiers used to identify common ancestors. An attacker capable of supplying deliberately crafted colliding files can cause different contents to produce the same recorded digest. The script may consequently skip synchronization because `lh == rh`, or it may incorrectly infer a common historical version and make an invalid conflict-resolution decision. This issue does not allow arbitrary MD5 preimages to be generated for any chosen existing file. Exploitation requires the attacker to prepare suitable collision material or otherwise control relevant file versions. Nevertheless, synchronization with untrusted endpoints makes collision resistance an appropriate security requirement. ### Attack Path 1. An attacker prepares two distinct files with the same MD5 digest. 2. The attacker arranges for different colliding versions to exist at the same relative path on the two synchronization endpoints, or introduces them into the recorded version history. 3. The victim executes ...[truncated 718 chars]
Remediation
<![CDATA[ ## Remediation Suggestions - Replace MD5 with a collision-resistant algorithm such as SHA-256 or BLAKE2b. ```python def file_hash(path): h = hashlib.sha256() with open(path, "rb") as f: while chunk := f.read(8192): h.update(chunk) return h.hexdigest() ``` - Version the synchronization-state format and record the hash algorithm alongside each digest. - Do not reinterpret existing MD5 values as SHA-256 values. Invalidate and rebuild old histories during migration. - Validate digest format and expected length when loading state. - If synchronization is performed across actively hostile endpoints and authenticity is required, use authenticated metadata or signed manifests in addition to collision-resistant content hashing. ]]>
Vulnerability Patterns
  • MCP Tool PoisoningHidden Instructions, Unicode Deception, Parameter Description Injection
  • Prompt InjectionInstruction Override, Hidden Instructions, Exfiltration Commands
  • Data ExfiltrationExternal Transmission, Env Variable Harvesting, File System Enumeration
  • Privilege EscalationExcessive Permissions, Sudo/Root Execution, Credential Access
  • Supply ChainUnpinned Dependencies, External Script Fetching, Obfuscated Code
Findings (2)

Intent-Code Divergence

Medium
Confidence
97% confidence
Finding
The section says `move_to_trash` is triggered 'only when both sides delete', but the very code snippet immediately below shows `move_to_trash` being called when only one side deleted and the other side is unchanged. This is an active contradiction in the file's own behavioral documentation, not merely an omission.

Natural-Language Policy Violations

Low
Confidence
90% confidence
Finding
This code file contains user- and maintainer-facing natural-language comments entirely in Chinese, including the behavioral description and sync rules. Under the policy rule, forcing a specific language without opt-in or justification is a locale/language policy concern.

Static analysis

No suspicious patterns detected.