diff --git a/.jules/sentinel.md b/.jules/sentinel.md index c5cbf40..fdb9705 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -17,3 +17,8 @@ **Vulnerability:** The `/radio/proxy/{stream_id}` endpoint instantiated an `httpx.AsyncClient` with `follow_redirects=True`. Although the initial user-provided URL was checked against SSRF using `validate_safe_url`, if the server responded with an HTTP redirect (e.g. 302 Found) to an internal URL, the client would follow it without re-validating the target URL, leading to an SSRF vulnerability. **Learning:** Checking a URL before making a request is insufficient if the HTTP client automatically follows redirects to new URLs. Each redirect target must also be validated against SSRF rules. **Prevention:** If `follow_redirects=True` must be used, add an `event_hooks={'request': [_validate_request_url]}` hook to `httpx.AsyncClient` to intercept and validate every outbound request URL during the lifecycle of the client, including redirects. + +## 2025-05-26 - [SSRF via Unvalidated Redirects in Pollers] +**Vulnerability:** Similar to the `proxy_stream` vulnerability, several backend poller modules (`gtfs_rt.py`, `alerts.py`, `utilities.py`, `news.py`, `weather.py`) used `httpx.AsyncClient(follow_redirects=True)` without validating the URLs of any HTTP redirects. This opened up the system to an SSRF vulnerability where external servers could redirect the poller to private/loopback IP addresses to probe the internal network. Also, the `0.0.0.0` address was not blocked which could resolve to localhost. +**Learning:** Checking for `ip.is_private`, `ip.is_loopback`, `ip.is_link_local`, and `ip.is_reserved` is insufficient on Linux. Attackers can provide `0.0.0.0` to bypass these checks, but Linux will route `0.0.0.0` to `localhost`. +**Prevention:** Implement an `event_hooks={'request': [validate_request_url]}` for all `httpx.AsyncClient` instances when `follow_redirects=True`. In addition, ensure that the centralized SSRF validation function blocks `ip.is_unspecified` (i.e. `0.0.0.0`) in addition to private/loopback/link-local/reserved addresses. diff --git a/backend/security.py b/backend/security.py index 710a978..8400bf1 100644 --- a/backend/security.py +++ b/backend/security.py @@ -66,5 +66,5 @@ def validate_webhook_url(url: str) -> None: def _reject_private_ip(ip: ipaddress.IPv4Address | ipaddress.IPv6Address) -> None: - if ip.is_loopback or ip.is_private or ip.is_link_local or ip.is_reserved: + if ip.is_loopback or ip.is_private or ip.is_link_local or ip.is_reserved or ip.is_unspecified: raise ValueError(f"URL resolves to a non-public address: {ip}") diff --git a/poller/pollers/alerts.py b/poller/pollers/alerts.py index f61a124..be3f63c 100644 --- a/poller/pollers/alerts.py +++ b/poller/pollers/alerts.py @@ -3,6 +3,7 @@ import feedparser import httpx from config import settings +from security import validate_request_url from bus import set_feed from .base import BasePoller @@ -112,7 +113,11 @@ async def poll(self): weather_alerts: list[dict] = [] # ── URL-based alert feeds ──────────────────────────────────────────── - async with httpx.AsyncClient(timeout=15, follow_redirects=True) as client: + async with httpx.AsyncClient( + timeout=15, + follow_redirects=True, + event_hooks={'request': [validate_request_url]} + ) as client: for feed in self._alert_feeds: try: resp = await client.get(feed["url"]) diff --git a/poller/pollers/gtfs_rt.py b/poller/pollers/gtfs_rt.py index e93a76e..0a6dcfe 100644 --- a/poller/pollers/gtfs_rt.py +++ b/poller/pollers/gtfs_rt.py @@ -9,6 +9,7 @@ import httpx +from security import validate_request_url from bus import get_bus, publish_entity from config import settings from .base import BasePoller @@ -144,7 +145,10 @@ async def _ensure_route_map(self, state: _FeedState) -> dict[str, dict]: params[feed.api_key_param] = feed.api_key try: - async with httpx.AsyncClient(timeout=90) as client: + async with httpx.AsyncClient( + timeout=90, + event_hooks={'request': [validate_request_url]} + ) as client: resp = await client.get( feed.static_gtfs_url, params=params, diff --git a/poller/pollers/news.py b/poller/pollers/news.py index 2ba4dc5..01c4fbc 100644 --- a/poller/pollers/news.py +++ b/poller/pollers/news.py @@ -5,6 +5,7 @@ import feedparser import httpx +from security import validate_request_url from bus import set_feed from .base import BasePoller @@ -80,7 +81,9 @@ async def poll(self): for src in self._rss_sources: try: async with httpx.AsyncClient( - timeout=15, follow_redirects=True + timeout=15, + follow_redirects=True, + event_hooks={'request': [validate_request_url]} ) as client: resp = await client.get(src["url"]) resp.raise_for_status() diff --git a/poller/pollers/utilities.py b/poller/pollers/utilities.py index a6f6ae1..aea6c25 100644 --- a/poller/pollers/utilities.py +++ b/poller/pollers/utilities.py @@ -1,5 +1,6 @@ import logging import httpx +from security import validate_request_url from bus import set_feed from .base import BasePoller @@ -28,7 +29,11 @@ async def poll(self): "outFields": "utilityName,metersOut,CountyName", "f": "json" } - async with httpx.AsyncClient(timeout=20, follow_redirects=True) as client: + async with httpx.AsyncClient( + timeout=20, + follow_redirects=True, + event_hooks={'request': [validate_request_url]} + ) as client: resp = await client.get(_ODIN_URL, params=params) resp.raise_for_status() data = resp.json() diff --git a/poller/pollers/weather.py b/poller/pollers/weather.py index c3b35c9..0e2f4b4 100644 --- a/poller/pollers/weather.py +++ b/poller/pollers/weather.py @@ -2,6 +2,7 @@ import logging import httpx from config import settings, load_regions +from security import validate_request_url from bus import set_feed from normalizers.weather import normalize_observation from .base import BasePoller @@ -109,7 +110,11 @@ async def _fetch_aqi(self) -> dict: } _success = False try: - async with httpx.AsyncClient(timeout=30, follow_redirects=True) as client: + async with httpx.AsyncClient( + timeout=30, + follow_redirects=True, + event_hooks={'request': [validate_request_url]} + ) as client: resp = await client.get(url, params=params) resp.raise_for_status() data = resp.json() diff --git a/poller/security.py b/poller/security.py new file mode 100644 index 0000000..22df2ca --- /dev/null +++ b/poller/security.py @@ -0,0 +1,58 @@ +import asyncio +import ipaddress +import socket +from urllib.parse import urlparse +import httpx + +async def validate_safe_url(url: str, allowed_schemes: set[str] | None = None) -> None: + """Raise ValueError if url is not a safe, public URL.""" + if allowed_schemes is None: + allowed_schemes = {"http", "https", "ws", "wss", "tcp"} + try: + parsed = urlparse(url) + except Exception as exc: + raise ValueError(f"Unparseable URL: {exc}") from exc + + if parsed.scheme not in allowed_schemes: + raise ValueError(f"URL scheme must be one of {allowed_schemes}") + + hostname = parsed.hostname or "" + if not hostname: + raise ValueError("URL has no hostname") + + await validate_safe_host(hostname) + + +async def validate_safe_host(hostname: str) -> None: + """Raise ValueError if hostname resolves to a non-public address.""" + if not hostname: + raise ValueError("No hostname provided") + try: + ip = ipaddress.ip_address(hostname) + _reject_private_ip(ip) + except ValueError as exc: + if "non-public address" in str(exc): + raise + # Not an IP literal — resolve via DNS and check each address + try: + loop = asyncio.get_running_loop() + infos = await loop.getaddrinfo(hostname, None) + except OSError as dns_exc: + raise ValueError(f"Cannot resolve hostname: {dns_exc}") from dns_exc + for info in infos: + addr = info[4][0] + _reject_private_ip(ipaddress.ip_address(addr)) + + +def _reject_private_ip(ip: ipaddress.IPv4Address | ipaddress.IPv6Address) -> None: + if ip.is_loopback or ip.is_private or ip.is_link_local or ip.is_reserved or ip.is_unspecified: + raise ValueError(f"URL resolves to a non-public address: {ip}") + + +async def validate_request_url(request: httpx.Request): + """Event hook for httpx.AsyncClient to validate outbound URLs.""" + try: + await validate_safe_url(str(request.url), allowed_schemes={"http", "https"}) + except ValueError as e: + # We raise a RequestError here so httpx catches it instead of crashing. + raise httpx.RequestError(f"SSRF validation failed: {e}", request=request)