Fix: DNS-based SSRF bypass in guardrail webhook URL validation

finbot/apps/labs/routes/guardrails.py

@@ -3,12 +3,16 @@
 from fastapi import APIRouter, Depends, HTTPException
 from pydantic import BaseModel, Field
 
-from finbot.core.auth.middleware import get_session_context
+from finbot.core.auth.middleware import (
+    get_authenticated_session_context,
+    get_session_context,
+)
 from finbot.core.auth.session import SessionContext
 from finbot.core.data.database import db_session
 from finbot.core.data.repositories import (
     CTFEventRepository,
     LabsGuardrailConfigRepository,
+    validate_webhook_url_async,
 )
 from finbot.guardrails.schemas import HookKind
 from finbot.guardrails.service import GuardrailHookService
@@ -60,9 +64,19 @@ async def get_guardrail_config(
 @router.put("", response_model=GuardrailConfigResponse, status_code=200)
 async def upsert_guardrail_config(
     body: GuardrailConfigRequest,
-    session_context: SessionContext = Depends(get_session_context),
+    session_context: SessionContext = Depends(get_authenticated_session_context),
 ):
     """Create or update the guardrail webhook configuration."""
+    # Validated here first, off the event loop with a bounded timeout --
+    # validate_webhook_url can perform a real DNS lookup, which is a
+    # synchronous, unbounded call with no built-in timeout. repo.upsert()
+    # is told to skip its own internal check below: re-running it would be
+    # a second, unbounded DNS resolution while holding an open DB session,
+    # which is worse than what we're fixing, not better.
+    valid, err = await validate_webhook_url_async(body.webhook_url)
+    if not valid:
+        raise HTTPException(status_code=422, detail=err)
+
     with db_session() as db:
         repo = LabsGuardrailConfigRepository(db, session_context)
         try:
@@ -71,6 +85,7 @@ async def upsert_guardrail_config(
                 hooks=body.hooks,
                 timeout_seconds=body.timeout_seconds,
                 enabled=body.enabled,
+                skip_url_validation=True,
             )
         except ValueError as exc:
             raise HTTPException(status_code=422, detail=str(exc)) from exc
@@ -82,7 +97,7 @@ async def upsert_guardrail_config(
 
 @router.post("/toggle", response_model=GuardrailConfigResponse)
 async def toggle_guardrail_enabled(
-    session_context: SessionContext = Depends(get_session_context),
+    session_context: SessionContext = Depends(get_authenticated_session_context),
 ):
     """Toggle the enabled flag on the guardrail config."""
     with db_session() as db:
@@ -99,7 +114,7 @@ async def toggle_guardrail_enabled(
 
 @router.post("/rotate-secret", response_model=GuardrailConfigResponse)
 async def rotate_signing_secret(
-    session_context: SessionContext = Depends(get_session_context),
+    session_context: SessionContext = Depends(get_authenticated_session_context),
 ):
     """Rotate the HMAC signing secret."""
     with db_session() as db:
@@ -116,7 +131,7 @@ async def rotate_signing_secret(
 
 @router.delete("", status_code=204)
 async def delete_guardrail_config(
-    session_context: SessionContext = Depends(get_session_context),
+    session_context: SessionContext = Depends(get_authenticated_session_context),
 ):
     """Delete the guardrail webhook configuration."""
     with db_session() as db:
@@ -130,7 +145,7 @@ async def delete_guardrail_config(
 
 @router.post("/test")
 async def test_webhook_delivery(
-    session_context: SessionContext = Depends(get_session_context),
+    session_context: SessionContext = Depends(get_authenticated_session_context),
 ):
     """Send a test before_tool hook to the user's webhook and return the result."""
     svc = GuardrailHookService(

finbot/core/data/repositories.py

@@ -1,8 +1,10 @@
 """Data Repositories for FinBot CTF Platform"""
 
+import asyncio
 import ipaddress
 import json
 import secrets
+import socket
 from datetime import UTC, datetime
 from urllib.parse import urlparse
 
@@ -1287,12 +1289,55 @@ def count_exfil_events(self) -> int:
     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("100.64.0.0/10"),  # RFC 6598 carrier-grade NAT -- used by
+    # several cloud providers for internal-only service traffic.
     ipaddress.ip_network("0.0.0.0/8"),
     ipaddress.ip_network("::1/128"),
+    ipaddress.ip_network("::/128"),  # unspecified IPv6; connect() to it behaves like
+    # loopback on some stacks, same reasoning as blocking 0.0.0.0/8 above.
     ipaddress.ip_network("fc00::/7"),
     ipaddress.ip_network("fe80::/10"),
 ]
 
+# Known, accepted gap: the deprecated IPv4-compatible IPv6 form (::127.0.0.1,
+# distinct from the IPv4-*mapped* ::ffff:127.0.0.1 form handled below) and the
+# NAT64 well-known prefix (64:ff9b::/96, which can embed an IPv4 address) are
+# not unwrapped here. Real-world exploitability is low -- the deprecated form
+# isn't routed by most modern kernels, and NAT64 requires the deployment to
+# actually run a NAT64 gateway -- but note it if extending this further.
+
+
+def _blocked_network_hit(addr: ipaddress.IPv4Address | ipaddress.IPv6Address) -> bool:
+    if isinstance(addr, ipaddress.IPv6Address) and addr.ipv4_mapped is not None:
+        addr = addr.ipv4_mapped
+    return any(addr in net for net in _BLOCKED_NETWORKS)
+
+
+def _hostname_resolves_to_blocked_address(hostname: str) -> str | None:
+    """Resolve hostname and check every returned address against
+    _BLOCKED_NETWORKS -- not just a literal IP typed directly into the URL.
+    Returns an error message if blocked/unresolvable, else None.
+
+    This catches DNS-based SSRF: a hostname that isn't itself an IP literal
+    but resolves to a loopback/private/link-local address (e.g. a domain an
+    attacker controls the DNS for, pointed at 127.0.0.1 or the cloud
+    metadata endpoint 169.254.169.254).
+    """
+    try:
+        addrinfo = socket.getaddrinfo(hostname, None)
+    except (socket.gaierror, OSError, UnicodeError):
+        # gaierror/OSError: resolution failure. UnicodeError: malformed
+        # hostname (e.g. an overlong label the idna codec rejects) --
+        # confirmed socket.getaddrinfo raises UnicodeEncodeError, not
+        # gaierror, for that case.
+        return f"Could not resolve hostname '{hostname}'"
+
+    for _family, _type, _proto, _canonname, sockaddr in addrinfo:
+        addr = ipaddress.ip_address(sockaddr[0])
+        if _blocked_network_hit(addr):
+            return f"Hostname '{hostname}' resolves to a blocked address ({sockaddr[0]})"
+    return None
+
 
 def validate_webhook_url(url: str) -> tuple[bool, str | None]:
     """Validate a webhook URL for safety.
@@ -1329,11 +1374,16 @@ def validate_webhook_url(url: str) -> tuple[bool, str | None]:
 
         try:
             addr = ipaddress.ip_address(hostname)
-            for net in _BLOCKED_NETWORKS:
-                if addr in net:
-                    return False, f"IP address {hostname} is in a blocked range"
         except ValueError:
-            pass
+            # Not a literal IP -- resolve it and check every address it
+            # points at, so a hostname can't be used to bypass the check
+            # a bare IP literal would have failed.
+            err = _hostname_resolves_to_blocked_address(hostname)
+            if err:
+                return False, err
+        else:
+            if _blocked_network_hit(addr):
+                return False, f"IP address {hostname} is in a blocked range"
 
     if not parsed.port and parsed.scheme == "https":
         pass
@@ -1345,6 +1395,33 @@ def validate_webhook_url(url: str) -> tuple[bool, str | None]:
     return True, None
 
 
+_WEBHOOK_VALIDATION_TIMEOUT_SECONDS = 2.0
+
+
+async def validate_webhook_url_async(
+    url: str, *, timeout: float = _WEBHOOK_VALIDATION_TIMEOUT_SECONDS
+) -> tuple[bool, str | None]:
+    """Async wrapper around validate_webhook_url for use from async call sites.
+
+    validate_webhook_url can perform a real DNS lookup (socket.getaddrinfo),
+    which is a synchronous, unbounded call with no built-in per-call timeout
+    in the stdlib. Called directly from an async context, a slow or
+    non-responding attacker-controlled DNS server would block the entire
+    event loop -- not just the caller's own request, every concurrent
+    session on that worker -- for as long as the OS resolver takes to give
+    up (often well past 30s). This offloads the check to a worker thread and
+    bounds the wait, so the event loop is freed up immediately on timeout
+    even though the worker thread itself may still be blocked on DNS in the
+    background (threads can't be forcibly killed in Python).
+    """
+    try:
+        return await asyncio.wait_for(
+            asyncio.to_thread(validate_webhook_url, url), timeout=timeout
+        )
+    except asyncio.TimeoutError:
+        return False, "Timed out validating webhook URL"
+
+
 # =============================================================================
 # Labs Guardrail Config Repository
 # =============================================================================
@@ -1379,14 +1456,26 @@ def upsert(
         hooks: dict[str, bool] | None = None,
         timeout_seconds: int = 5,
         enabled: bool = True,
+        *,
+        skip_url_validation: bool = False,
     ) -> tuple[LabsGuardrailConfig, bool]:
         """Create or update guardrail config for the current user.
 
+        skip_url_validation: set when the caller has already validated
+        webhook_url itself (e.g. via the async pre-check in the route
+        handler, which is timeout-bounded off the event loop). Re-running
+        the synchronous check here would perform a second, unbounded DNS
+        resolution while holding an open DB session -- worse than a bare
+        blocking call, and pure redundant work for a caller that already
+        validated. Callers that have NOT already validated (any other/future
+        caller of this repository method) get the check by default.
+
         Returns (config, created) where created=True if a new row was inserted.
         """
-        valid, err = validate_webhook_url(webhook_url)
-        if not valid:
-            raise ValueError(err)
+        if not skip_url_validation:
+            valid, err = validate_webhook_url(webhook_url)
+            if not valid:
+                raise ValueError(err)
 
         if hooks is not None:
             unknown = set(hooks.keys()) - self.VALID_HOOK_KINDS

finbot/guardrails/service.py

@@ -19,7 +19,10 @@
 from finbot.core.auth.session import SessionContext
 from finbot.core.data.database import db_session
 from finbot.core.data.models import LabsGuardrailConfig
-from finbot.core.data.repositories import LabsGuardrailConfigRepository
+from finbot.core.data.repositories import (
+    LabsGuardrailConfigRepository,
+    validate_webhook_url_async,
+)
 from finbot.core.messaging import event_bus
 from finbot.guardrails.schemas import (
     HookEnvelope,
@@ -145,6 +148,14 @@ async def invoke(
         error_detail: str | None = None
 
         try:
+            # Re-validated here (not just at registration time) as a second
+            # layer against DNS rebinding: a hostname that resolved to a
+            # public address when the webhook was registered could resolve
+            # to an internal address by the time this hook actually fires.
+            valid, err = await validate_webhook_url_async(config.webhook_url)
+            if not valid:
+                raise ValueError(err)
+
             async with httpx.AsyncClient() as client:
                 resp = await client.post(
                     config.webhook_url,