T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/memoryos_admin.py:35
- Finding
- Path Traversal Through Unsanitized User and Assistant Identifiers<![CDATA[ ## Vulnerability Details **File Location**: `scripts/memoryos_admin.py`, lines 35-40 **Vulnerability Type**: Path traversal and insufficient filesystem boundary validation **Risk Level**: Medium ### Vulnerable Code ```python def user_file(data_root: Path, user_id: str) -> Path: return data_root / "users" / user_id / "long_term_user.json" def assistant_file(data_root: Path, assistant_id: str) -> Path: return data_root / "assistants" / assistant_id / "long_term_assistant.json" ``` ### Technical Analysis The `user_file()` and `assistant_file()` functions directly incorporate command-line-controlled identifiers into filesystem paths. Neither function validates the identifiers or verifies that the resulting paths remain beneath the intended `users` or `assistants` directory. An identifier can contain absolute paths, path separators, or parent-directory components such as `..`. Python's `pathlib` path composition preserves these traversal semantics. Absolute path components may also discard the preceding base path. The resulting untrusted paths are subsequently used by operations that read, create, overwrite, export, or back up MemoryOS data. The affected commands include: - `summary` - `backup` - `search-user` - `search-assistant` - `add-user-knowledge` - `add-assistant-knowledge` - `set-profile` - `export-markdown` For read operations, exploitation requires a targeted file with the expected fixed filename, such as `long_term_user.json` or `long_term_assistant.json`, and valid JSON content where parsing is performed. For write operations, an attacker can create the expected filename in a reachable directory or modify an existing compatible JSON file. Access remains limited to the permissions of the account running the script. Path resolution checks alone are insufficient if writable path components can be replaced with symbolic links. Where tenant or filesystem isolation is required, symlink handling must therefore also be hardened. ## ...[truncated 1811 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Apply a strict identifier allowlist** Permit only characters needed by the identifier format, such as letters, numbers, periods, underscores, and hyphens. Explicitly reject empty identifiers, `.` and `..`. ```python import re ID_PATTERN = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]*$") def validate_id(value: str, field_name: str) -> str: if not ID_PATTERN.fullmatch(value) or value in {".", ".."}: raise ValueError(f"Invalid {field_name}") return value ``` 2. **Enforce containment after canonical path resolution** Resolve the expected base directory and candidate path, then confirm that the candidate remains beneath the expected base. ```python def confined_file(base: Path, identifier: str, filename: str) -> Path: identifier = validate_id(identifier, "identifier") resolved_base = base.resolve() candidate = (resolved_base / identifier / filename).resolve() if not candidate.is_relative_to(resolved_base): raise ValueError("Resolved path escapes the permitted directory") return candidate ``` Use separate bases for users and assistants: ```python def user_file(data_root: Path, user_id: str) -> Path: return confined_file(data_root / "users", user_id, "long_term_user.json") def assistant_file(data_root: Path, assistant_id: str) -> Path: return confined_file( data_root / "assistants", assistant_id, "long_term_assistant.json", ) ``` 3. **Harden against symbolic-link escapes** If directories may be writable by untrusted accounts, reject symbolic links in each existing path component or use descriptor-relative filesystem APIs with no-follow behavior. Revalidate containment immediately before sensitive reads and writes to reduce time-of-check/time-of-use exposure. 4. **Use least-privilege filesystem permissions** Run the administrative script under an a ...[truncated 797 chars]