T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/bookmark_manager.py:116
- Finding
- Stored HTML Injection in Bookmark Export## Vulnerability Details **File Location**: `scripts/bookmark_manager.py`, lines 116-120 **Vulnerability Type**: Stored HTML injection caused by unescaped bookmark data **Risk Level**: Medium ### Vulnerable Code ```python elif fmt == "html": lines = ["<html><body><ul>"] for b in data["urls"]: lines.append(f'<li><a href="{b["url"]}">{b["title"]}</a></li>') lines.append("</ul></body></html>") print("\n".join(lines)) ``` ### Technical Analysis The HTML exporter inserts the stored URL and title directly into an HTML attribute and text context without context-appropriate escaping. Both values can originate from command-line input supplied to the `add` command. An attacker-controlled title can inject HTML elements because characters such as `<`, `>`, `&`, and quotes are preserved. A crafted URL can contain a quote that terminates the `href` attribute and introduces additional attributes or markup. Prefix normalization to `http://` or `https://` does not prevent attribute injection because the remainder of the string is not parsed or validated as a well-formed URL. The vulnerability becomes exploitable when the generated output is saved as an HTML document and opened in a browser or rendered by another HTML consumer. ### Attack Path 1. An attacker persuades the user to add a bookmark containing a malicious title or crafted URL. 2. The `add` command stores the value in `assets/bookmarks.json` without HTML-specific validation. 3. The user runs `bookmark_manager.py export html`. 4. The exporter embeds the malicious value directly into the generated markup. 5. The user saves or redirects the output to an HTML file and opens it in a browser. 6. The injected markup or event-handler content executes or alters the rendered document. ### Impact Assessment Exploitation can modify the exported document, create deceptive links or forms, and execute browser-side script in the generated document's lo ...[truncated 260 chars]
- Remediation
- ## Remediation Suggestions - Escape titles with `html.escape(title, quote=True)` before inserting them into HTML text. - Escape URL attribute values with `html.escape(url, quote=True)`. - Parse URLs with a standard URL parser and accept only well-formed `http` and `https` URLs. - Reject control characters and malformed authority or host components. - Prefer a trusted HTML templating mechanism with automatic contextual escaping. - Add tests covering titles and URLs containing quotes, angle brackets, ampersands, event-handler fragments, and line breaks. Example hardening: ```python import html from urllib.parse import urlparse parsed = urlparse(b["url"]) if parsed.scheme not in {"http", "https"} or not parsed.netloc: raise ValueError("Invalid bookmark URL") safe_url = html.escape(b["url"], quote=True) safe_title = html.escape(b["title"], quote=True) lines.append(f'<li><a href="{safe_url}">{safe_title}</a></li>') ```