diff options
| author | Christophe Besson <cbesson@gmail.com> | 2026-09-01 19:09:20 +0200 |
|---|---|---|
| committer | Christophe Besson <cbesson@gmail.com> | 2026-09-01 19:09:20 +0200 |
| commit | 0808d594a371e7caea54f45a71db586160ae75ad (patch) | |
| tree | beb657f23cabb336ba4301e3438c4094b5fef452 /packages/meshbay-node/src/meshbay_node/linkpreview.py | |
| parent | 8685ec09a658157f005925f54832e7a846b5366b (diff) | |
| download | meshbay-0808d594a371e7caea54f45a71db586160ae75ad.tar.gz | |
fix(node): bound and tighten the chat link-preview SSRF surface
The link-preview fetch is an outbound request to an address a member
chose. safe_url() already blocked non-public addresses and re-checked
each redirect hop; this adds the parts that were missing:
- Rate limit. `_do_link_preview_request` was reachable by any member
with no ceiling, so a member — or a hub minting tokens for many
accounts — could drive unbounded outbound HTTP from the operator's
machine (amplification / DoS / on-demand IP disclosure to arbitrary
hosts). Now bounded per connection (15) and node-wide (60) over a
60 s window; only a real fetch counts, a cache hit is free, and over
the ceiling the reply is a plain `ok: false` (bare link), not cached.
- Port allowlist. safe_url() passed `parts.port` straight through, so
a member could aim the node at `http://<public-host>:<any-port>`.
Restricted to {80, 443, 8080, 8443} — every real OpenGraph page,
none of SSH / mail / DB / cache / search / admin ports.
- DNS rebinding. The connection's actual peer address is now
re-checked against the public-address rule (`_reject_if_rebound`),
so a name that resolves clean and then to something internal does
not get its body read. Best-effort (no `network_stream` extension,
no check); a full literal-pin is noted as remaining hardening.
- Decompression bomb. `_downscale` now refuses an image whose header
dimensions exceed ~40 MP before convert()/thumbnail() decode it.
Third security review, finding M3.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011pG75yGK3NthNfyjH74omG
Diffstat (limited to 'packages/meshbay-node/src/meshbay_node/linkpreview.py')
| -rw-r--r-- | packages/meshbay-node/src/meshbay_node/linkpreview.py | 51 |
1 files changed, 45 insertions, 6 deletions
diff --git a/packages/meshbay-node/src/meshbay_node/linkpreview.py b/packages/meshbay-node/src/meshbay_node/linkpreview.py index 64067d1..b223aea 100644 --- a/packages/meshbay-node/src/meshbay_node/linkpreview.py +++ b/packages/meshbay-node/src/meshbay_node/linkpreview.py @@ -14,12 +14,15 @@ hub: Because the node makes an outbound request to an address a *member* chose, this is an SSRF surface. `safe_url()` is the gate: http(s) only, no -credentials, and the resolved address must be globally routable — no -loopback, private, link-local, multicast or reserved range. Redirects are -followed by hand so every hop is re-checked. Residual: a DNS name that -resolves clean here and to something internal microseconds later at connect -time (rebinding) — narrow, and closed properly by pinning the checked IP, -which is a follow-up. +credentials, the port restricted to the web set, and every resolved address +must be globally routable — no loopback, private, link-local, multicast or +reserved range. Redirects are followed by hand so every hop is re-checked, +and the address the connection actually landed on is re-checked against the +same rule (`_reject_if_rebound`), so a name that resolves clean and then to +something internal (rebinding) does not get its body read. A full pin — +connect to the validated literal, verify the certificate for the name — is +the remaining hardening. How many previews a member can trigger is +rate-limited by the caller (`_do_link_preview_request`). Nothing is stored durably: the caller keeps an in-memory TTL cache and the OG image rides the existing `media_cache` thumb store (same as a poster). @@ -42,9 +45,15 @@ _TIMEOUT = 5.0 _MAX_REDIRECTS = 3 _MAX_HTML_BYTES = 512 * 1024 _MAX_IMAGE_BYTES = 2 * 1024 * 1024 +_MAX_IMAGE_PIXELS = 40_000_000 # ~40 MP; an OG card image is a fraction of this _IMAGE_MAX_DIM = 600 _UA = "MeshBayBot/1.0 (+https://meshbay.org; link preview)" +# Ports a real OpenGraph-bearing page is served on. Everything else — SSH, mail, +# databases, caches, search, admin panels — is refused, so a member cannot aim +# the node at an arbitrary service even on a public host. +_ALLOWED_PORTS = frozenset({80, 443, 8080, 8443}) + class UnsafeURL(ValueError): """The URL points somewhere the node must not fetch from.""" @@ -75,6 +84,12 @@ def safe_url(url: str) -> str: host = parts.hostname if not host: raise UnsafeURL("no host") + try: + port = parts.port + except ValueError: + raise UnsafeURL("bad port") + if port is not None and port not in _ALLOWED_PORTS: + raise UnsafeURL(f"port {port}") # An IP literal is checked directly; a name is resolved and every answer # must be public — a hostname with one public and one 127.0.0.1 record # would otherwise be a way in. @@ -136,12 +151,32 @@ def _first(metas: dict[str, str], *keys: str) -> str | None: return None +def _reject_if_rebound(resp: httpx.Response) -> None: + """ + `safe_url` validated the name's addresses; this checks the one the + connection actually landed on, so a name that resolves clean and then to + something internal (DNS rebinding) does not get its body read. + + Best-effort: the `network_stream` extension is not present on every + transport (a MockTransport in tests has none), and its absence is not a + failure — the pre-check and the per-hop redirect re-check still stand. + """ + try: + stream = resp.extensions.get("network_stream") + addr = stream.get_extra_info("server_addr") if stream else None + except Exception: + return + if addr and not _addr_is_public(str(addr[0])): + raise UnsafeURL(f"connected to non-public address {addr[0]}") + + async def _get(client: httpx.AsyncClient, url: str) -> httpx.Response: """One GET with manual, re-validated redirects.""" current = safe_url(url) for _ in range(_MAX_REDIRECTS + 1): resp = await client.get(current, headers={"User-Agent": _UA}, follow_redirects=False) + _reject_if_rebound(resp) if resp.is_redirect and "location" in resp.headers: current = safe_url(urljoin(current, resp.headers["location"])) continue @@ -242,6 +277,10 @@ def _downscale(raw: bytes) -> bytes | None: return None try: with Image.open(BytesIO(raw)) as im: + # The header is parsed but the pixels are not decoded yet — refuse a + # decompression bomb before convert()/thumbnail() allocate for it. + if im.width * im.height > _MAX_IMAGE_PIXELS: + return None im = im.convert("RGB") im.thumbnail((_IMAGE_MAX_DIM, _IMAGE_MAX_DIM)) out = BytesIO() |