T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/server.py:110
- Finding
- Path Allowlist Bypass Permits Operations Outside Approved Directories<![CDATA[ ## Vulnerability Details **File Location**: `scripts/server.py:110-116`, `scripts/server.py:150-154`, and `scripts/server.py:177-182` **Vulnerability Type**: Incomplete path validation and allowlist bypass **Risk Level**: High ### Vulnerable Code ```python ALLOWED_PATHS = [p for p in os.environ.get("ARCPY_ALLOWED_PATHS", "C:/GIS-AI-Course/;C:\\GIS-AI-Course\\").split(";") if p.strip()] def check_path(p): if not p or not isinstance(p, str): return True p = p.replace("\\", "/").lower() return any(p.startswith(a.replace("\\", "/").lower()) for a in ALLOWED_PATHS) ``` ```python # Path security check for k, v in args.items(): if isinstance(v, str) and ("/" in v or "\\" in v) and (".shp" in v.lower() or ".tif" in v.lower() or ".gdb" in v.lower()): if not check_path(v): self.json({"status": "error", "message": f"Path not allowed: {v}"}, 403) return ``` ```python # Parse kwargs: handle both flat args and JSON string kwargs if "__kwargs" in args and len(args) == 1: kwargs = json.loads(args["__kwargs"]) else: kwargs = {k: v for k, v in args.items() if not k.startswith("__")} # Execute! result = func(**kwargs) ``` ### Technical Analysis The server claims to enforce a path allowlist, but the validation is shallow and based on string heuristics rather than canonical filesystem paths. The implementation has several bypass conditions: 1. **Nested values are not inspected.** Only top-level string values in `args` are validated. Lists, dictionaries, and other nested structures are ignored. ArcPy tools commonly accept lists of input datasets, such as the `in_features` argument for intersection and merge operations. 2. **Only three path patterns trigger validation.** A string is checked only if it contains `.shp`, `.tif`, or `.gdb`. ArcPy supports many additional path-bearing resource types, including CSV files, text files, XML files, geodatabases using other formats, database connections, layer ...[truncated 2514 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Construct the final `kwargs` first and recursively inspect every list, tuple, dictionary, and string before invoking ArcPy. 2. Canonicalize local paths with `pathlib.Path.resolve(strict=False)` or an equivalent Windows-aware mechanism. 3. Verify containment using `os.path.commonpath` or resolved `Path` parents rather than `startswith`. 4. Reject traversal components, ambiguous relative paths, device paths, UNC paths, alternate data streams, and unsupported URI schemes unless explicitly required. 5. Require absolute paths and resolve them against a controlled base directory. 6. Obtain ArcPy parameter metadata and validate every path-bearing parameter instead of detecting paths through three filename extensions. 7. Maintain separate read and write allowlists. Output paths should receive stricter validation than input paths. 8. Revalidate every path immediately before tool execution, including paths embedded in composite parameters. 9. Normalize administrator-supplied allowed roots and enforce directory-boundary semantics. 10. Add regression tests for nested lists, dictionaries, traversal paths, sibling-prefix paths, relative paths, mixed separators, case variations, UNC paths, and all supported dataset formats. ]]>