From a1dd84c00e411d548a6bb25831ee8aefa902f99b Mon Sep 17 00:00:00 2001 From: elhoim Date: Mon, 31 Aug 2026 00:24:20 +0000 Subject: [PATCH] fix: [html_to_markdown] close SSRF holes in the URL safety check The module guards fetchHTML with is_safe_url, but that guard leaks in two ways. First, the destination is validated once and then requests.get follows redirects on its own, so a public host answering 302 Location: http://127.0.0.1:6379/ makes the module fetch an internal service and hand its response back to the analyst as page content. Second, the hand-rolled BLOCKED_RANGES list only covers 127/8, RFC1918, 169.254/16 and ::1, so http://0.0.0.0/, http://[fe80::1]/, http://[fc00::1]/ and the shared address space 100.64.0.0/10 pass the check outright. Redirects are now followed manually, revalidating every hop and capping the chain, and the range list is replaced by ipaddress' is_global, which covers every range the old list missed (it also rejects multicast and benchmark addresses, which are equally non-public destinations). Rebinding between the check and the connection still needs connection-level IP pinning and is not addressed here. Verified with flake8 on the changed file and the full suite against a local modules server: 161 passed, 4 skipped, 5 subtests passed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018dfYpyaSZd1nxSRLr8suj8 --- .../modules/expansion/html_to_markdown.py | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/misp_modules/modules/expansion/html_to_markdown.py b/misp_modules/modules/expansion/html_to_markdown.py index 55d5802b1..67c286832 100755 --- a/misp_modules/modules/expansion/html_to_markdown.py +++ b/misp_modules/modules/expansion/html_to_markdown.py @@ -3,7 +3,7 @@ import requests import ipaddress -from urllib.parse import urlparse +from urllib.parse import urljoin, urlparse from bs4 import BeautifulSoup from markdownify import markdownify @@ -27,14 +27,7 @@ } -BLOCKED_RANGES = [ - ipaddress.ip_network("127.0.0.0/8"), - ipaddress.ip_network("10.0.0.0/8"), - ipaddress.ip_network("172.16.0.0/12"), - ipaddress.ip_network("192.168.0.0/16"), - ipaddress.ip_network("169.254.0.0/16"), - ipaddress.ip_network("::1/128"), -] +MAX_REDIRECTS = 5 def _normalize_ip_address(ip_str: str) -> ipaddress.IPv4Address | ipaddress.IPv6Address: @@ -43,8 +36,7 @@ def _normalize_ip_address(ip_str: str) -> ipaddress.IPv4Address | ipaddress.IPv6 def _is_ip_blocked(ip_str: str) -> bool: - ip = _normalize_ip_address(ip_str) - return any(ip in net for net in BLOCKED_RANGES) + return not _normalize_ip_address(ip_str).is_global def _hostname_resolves_to_blocked_ip(hostname: str) -> bool: @@ -66,10 +58,15 @@ def is_safe_url(url: str) -> bool: def fetchHTML(url): - if not is_safe_url(url): - raise ValueError(f"Blocked URL: {url}") - r = requests.get(url, timeout=10) - return r.text + for _ in range(MAX_REDIRECTS + 1): + if not is_safe_url(url): + raise ValueError(f"Blocked URL: {url}") + r = requests.get(url, timeout=10, allow_redirects=False) + if r.is_redirect and r.headers.get("location"): + url = urljoin(url, r.headers["location"]) + continue + return r.text + raise ValueError(f"Too many redirects: {url}") def stripUselessTags(html):