T09 · Insecure Skill Coding Practices
- Location
references/data-storage.md:183- Finding
User-Controlled Path Construction Enables Path Traversal and Cross-User File Access
- Content
View full analysis
Vulnerability Details
File Location:
references/data-storage.md:183,references/data-storage.md:213,references/data-storage.md:261,references/data-storage.md:317,references/data-storage.md:381, andreferences/data-storage.md:696-705
Vulnerability Type: Path traversal and broken access control
Risk Level: HighVulnerable Code
python file_path = os.path.join(USER_PROFILES_DIR, f"{user_id}.json")python file_path = os.path.join(SIP_PLANS_DIR, f"{user_id}_{plan_id}.json")python file_path = os.path.join(INVESTMENT_DIARY_DIR, f"{user_id}_diary_{diary_id}.json")python def check_file_access(file_path: str, user_id: str) -> bool: """Check file access permissions.""" # The filename should contain user_id if user_id not in file_path: return False # Check whether the file exists if not os.path.exists(file_path): return False return TrueTechnical Analysis
The documented storage functions interpolate caller-controlled
user_idandplan_idvalues directly into filesystem paths. The code does not reject absolute paths, directory separators,..components, control characters, or other unsafe filename content.os.path.join()does not enforce containment. A crafted identifier can introduce traversal components, while an absolute path component can cause preceding path components to be discarded in applicable constructions. The resulting path is then passed to generic JSON read or write functions.The proposed
check_file_access()function does not provide effective authorization. It only checks whether the textualuser_idappears somewhere in the path and whether the file exists. It does not:- Canonicalize the path.
- Verify that the resolved path remains under the expected storage root.
- Establish that the authenticated caller owns the requested record.
- Prevent one user id ...[truncated 1545 chars]
- Remediation
View remediation
Remediation Suggestions
-
Do not use caller-provided identifiers directly as filenames. Map authenticated account identifiers to server-generated opaque identifiers such as UUIDs.
-
Apply a strict allowlist to every path component, for example
^[A-Za-z0-9_-]{1,64}$. -
Resolve and validate every path before access:
python from pathlib import Path ROOT = Path("/var/lib/fund-advisor/user_profiles").resolve() def safe_profile_path(user_id: str) -> Path: if not re.fullmatch(r"[A-Za-z0-9_-]{1,64}", user_id): raise ValueError("Invalid user identifier") candidate = (ROOT / f"{user_id}.json").resolve() if ROOT not in candidate.parents: raise PermissionError("Path escapes the storage root") return candidate -
Derive the effective user identity from authenticated session context rather than accepting arbitrary
user_idvalues from Agent tool arguments. -
Enforce record ownership for every create, read, update, and delete operation.
-
Reject symbolic links or use safe descriptor-based file operations where the platform supports them.
-
Add tests for
../, absolute paths, mixed separators, symbolic links, Unicode separator variants, and identifiers that are substrings of other identifiers. -
Remove the substring-based
check_file_access()implementation and replace it with canonical containment plus authenticated ownership validation.
-