T09 · Insecure Skill Coding Practices
Warning
- Location
- tarot_skill/scripts/generate_spread_image.py:89
- Finding
- Unrestricted Image Source Handling Enables SSRF and Local File Disclosure<![CDATA[ ## Vulnerability Details **File Location**: `tarot_skill/scripts/generate_spread_image.py`, lines 89–95 and 138–173 **Vulnerability Type**: Server-Side Request Forgery and arbitrary local image-file read **Risk Level**: Medium ### Vulnerable Code ```python def fetch_image(url: str) -> bytes: if "://" in url and not url.startswith("file:"): req = urllib.request.Request(url, headers=BROWSER_HEADERS) with urllib.request.urlopen(req, timeout=TIMEOUT) as r: return r.read() path = url.replace("file:", "").strip() return Path(path).read_bytes() ``` The attacker-controlled value reaches this function through the reading JSON: ```python raw = Path(args.input).read_text(encoding="utf-8") if args.input else sys.stdin.read() reading = json.loads(raw) if not reading.get("ok") or not reading.get("spreadType") or not reading.get("spread"): print("Invalid reading: need ok=true, spreadType, and spread array.", file=sys.stderr) sys.exit(1) spread_type = reading["spreadType"] layout = SPREAD_LAYOUTS.get(spread_type) if not layout or len(layout["slots"]) != len(reading["spread"]): print( f'Layout for "{spread_type}" has {len(layout["slots"]) if layout else 0} slots, ' f"but spread has {len(reading['spread'])} cards.", file=sys.stderr, ) sys.exit(1) cw = layout["canvas_width"] ch = layout["canvas_height"] bg_hex = layout["background_color"].lstrip("#") bg = tuple(int(bg_hex[i : i + 2], 16) for i in (0, 2, 4)) base = Image.new("RGB", (cw, ch), bg) images_dir = Path(args.images_dir) if args.images_dir else None if images_dir is None: default_cards = _ROOT / "cards" _ensure_card_images_once(default_cards) if default_cards.is_dir(): images_dir = default_cards for i, pos in enumerate(reading["spread"]): slot = layout["slots"][i] card_id = pos["card"]["id"] fallback_url = pos["card"].get("image") or get_card_image_url(card_id) url = get_card_url(card_i ...[truncated 3346 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Do not trust `card.image` from reading JSON. Resolve artwork exclusively from a validated card ID and a server-controlled mapping. 2. Validate card IDs against the known 78-card dataset before constructing an image location. 3. If remote retrieval remains necessary, allow only HTTPS URLs under the exact approved artwork hostname and expected path prefix. 4. Resolve DNS and reject loopback, private, link-local, multicast, reserved, and metadata-service addresses for both IPv4 and IPv6. 5. Disable redirects or validate every redirect destination using the same scheme, hostname, path, and IP-address restrictions. 6. For local artwork, resolve the candidate path and verify with `Path.is_relative_to()` or an equivalent containment check that it remains beneath the configured image directory. 7. Reject `file:` URLs, absolute paths, traversal components, and arbitrary fallback paths. 8. Apply a strict maximum download size while streaming instead of calling `read()` without a limit. 9. Verify the response content type and reject non-image responses before decoding. 10. Configure Pillow pixel limits, catch decompression-bomb errors, and enforce maximum source dimensions. 11. Run image generation in a sandbox with restricted filesystem access and no internal-network access. 12. Add regression tests covering loopback URLs, metadata addresses, redirects, IPv6 private addresses, `file:` URLs, absolute paths, and directory traversal. ]]>