aboutsummaryrefslogtreecommitdiffstats
path: root/packages/meshbay-node
diff options
context:
space:
mode:
authorChristophe Besson <cbesson@gmail.com>2026-09-01 19:09:20 +0200
committerChristophe Besson <cbesson@gmail.com>2026-09-01 19:09:20 +0200
commit0808d594a371e7caea54f45a71db586160ae75ad (patch)
treebeb657f23cabb336ba4301e3438c4094b5fef452 /packages/meshbay-node
parent8685ec09a658157f005925f54832e7a846b5366b (diff)
downloadmeshbay-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')
-rw-r--r--packages/meshbay-node/src/meshbay_node/linkpreview.py51
-rw-r--r--packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py40
-rw-r--r--packages/meshbay-node/tests/test_link_preview_request.py50
-rw-r--r--packages/meshbay-node/tests/test_linkpreview.py40
4 files changed, 173 insertions, 8 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()
diff --git a/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py b/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py
index 003cd23..64ba75c 100644
--- a/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py
+++ b/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py
@@ -143,6 +143,16 @@ def _link_preview_cache_put(url: str, value: dict) -> None:
_link_preview_cache.pop(oldest, None)
_link_preview_cache[url] = (time.time(), value)
+
+# A member pasting a link is normal; a member — or a hub minting tokens for many
+# accounts — firing hundreds is amplification/DoS and a way to make the node
+# reach arbitrary hosts on demand (finding M3). Only a real outbound fetch is
+# counted (a cache hit costs nothing), and the ceilings are generous enough that
+# ordinary chat never meets them.
+_LINK_PREVIEW_RATE_WINDOW = 60.0
+_LINK_PREVIEW_RATE_PER_CONN = 15
+_LINK_PREVIEW_RATE_NODE = 60
+
# Upload limits (finding C5a). Uploads used to land directly in the shared root under
# a name the client chose, overwriting whatever was already there — which both violated
# node sovereignty and defeated the delete authorization (overwrite a file, become its
@@ -3588,6 +3598,27 @@ class WebRTCPeerSession:
],
})
+ def _link_preview_rate_ok(self) -> bool:
+ """
+ True when this preview fetch is within both the per-connection and the
+ node-wide window; records it when so, and both counts are trimmed to the
+ window on every call so the lists cannot grow without bound.
+ """
+ now = time.monotonic()
+ w = _LINK_PREVIEW_RATE_WINDOW
+ mine = [t for t in getattr(self, "_link_preview_hits", []) if now - t < w]
+ node = [t for t in self._ctx.get("link_preview_hits", []) if now - t < w]
+ if (len(mine) >= _LINK_PREVIEW_RATE_PER_CONN
+ or len(node) >= _LINK_PREVIEW_RATE_NODE):
+ self._link_preview_hits = mine
+ self._ctx["link_preview_hits"] = node
+ return False
+ mine.append(now)
+ node.append(now)
+ self._link_preview_hits = mine
+ self._ctx["link_preview_hits"] = node
+ return True
+
async def _do_link_preview_request(self, msg: dict) -> None:
"""
Unfurl a URL a member pasted into chat (draft-v6 §2.7 enrichment rule:
@@ -3608,6 +3639,15 @@ class WebRTCPeerSession:
self._send({**cached, "type": MNP.LINK_PREVIEW_RESP, "v": MNP_VERSION})
return
+ if not self._link_preview_rate_ok():
+ # Same shape as any other miss — the client shows the bare link. A
+ # rate-limited result is not cached, so it is retried once the
+ # window clears rather than pinned as "no preview".
+ log.debug("link_preview_req: rate-limited (peer=%s)", self._peer_id)
+ self._send({"type": MNP.LINK_PREVIEW_RESP, "v": MNP_VERSION,
+ "url": key, "ok": False})
+ return
+
resp: dict = {"type": MNP.LINK_PREVIEW_RESP, "v": MNP_VERSION,
"url": key, "ok": False}
try:
diff --git a/packages/meshbay-node/tests/test_link_preview_request.py b/packages/meshbay-node/tests/test_link_preview_request.py
index d975f91..fe7dec6 100644
--- a/packages/meshbay-node/tests/test_link_preview_request.py
+++ b/packages/meshbay-node/tests/test_link_preview_request.py
@@ -34,9 +34,10 @@ def _clear_cache():
webrtc_server._link_preview_cache.clear()
-def _session(media_cache):
+def _session(media_cache, ctx=None):
s = WebRTCPeerSession.__new__(WebRTCPeerSession)
- s._ctx = {"media_cache": media_cache}
+ s._ctx = ctx if ctx is not None else {"media_cache": media_cache}
+ s._peer_id = "t"
s.sent = []
s._send = s.sent.append
return s
@@ -79,6 +80,51 @@ async def test_unfurlable_failure_is_ok_false(media_cache, monkeypatch):
assert "image_thumb_hash" not in resp
+async def test_rate_limit_per_connection(media_cache, monkeypatch):
+ """A member firing many previews is bounded; over the ceiling the reply is
+ a plain `ok: false` (bare link) and no outbound fetch is made."""
+ monkeypatch.setattr(webrtc_server, "_LINK_PREVIEW_RATE_PER_CONN", 3)
+ calls = {"n": 0}
+
+ async def counting_preview(url, **k):
+ calls["n"] += 1
+ return {"url": url, "title": "x", "description": None,
+ "site_name": None, "image_url": None}
+ monkeypatch.setattr(linkpreview, "fetch_preview", counting_preview)
+
+ s = _session(media_cache)
+ for i in range(3):
+ await s._do_link_preview_request({"url": f"https://example.com/{i}"})
+ assert calls["n"] == 3
+ assert all(r["ok"] for r in s.sent)
+
+ await s._do_link_preview_request({"url": "https://example.com/over"})
+ assert calls["n"] == 3 # not fetched
+ assert s.sent[-1]["ok"] is False
+
+
+async def test_rate_limit_is_node_wide(media_cache, monkeypatch):
+ """Two connections share the node-wide ceiling."""
+ monkeypatch.setattr(webrtc_server, "_LINK_PREVIEW_RATE_PER_CONN", 100)
+ monkeypatch.setattr(webrtc_server, "_LINK_PREVIEW_RATE_NODE", 2)
+ calls = {"n": 0}
+
+ async def counting_preview(url, **k):
+ calls["n"] += 1
+ return {"url": url, "title": "x", "description": None,
+ "site_name": None, "image_url": None}
+ monkeypatch.setattr(linkpreview, "fetch_preview", counting_preview)
+
+ ctx = {"media_cache": media_cache}
+ a, b = _session(media_cache, ctx), _session(media_cache, ctx)
+ await a._do_link_preview_request({"url": "https://example.com/a"})
+ await b._do_link_preview_request({"url": "https://example.com/b"})
+ await b._do_link_preview_request({"url": "https://example.com/c"})
+
+ assert calls["n"] == 2
+ assert b.sent[-1]["ok"] is False
+
+
async def test_second_request_for_the_same_url_is_served_from_cache(media_cache, monkeypatch):
calls = {"n": 0}
diff --git a/packages/meshbay-node/tests/test_linkpreview.py b/packages/meshbay-node/tests/test_linkpreview.py
index 0dd951a..a14b173 100644
--- a/packages/meshbay-node/tests/test_linkpreview.py
+++ b/packages/meshbay-node/tests/test_linkpreview.py
@@ -48,6 +48,30 @@ def test_safe_url_refuses(url):
safe_url(url)
+@pytest.mark.parametrize("url", [
+ "http://example.com:22/x", # SSH
+ "http://example.com:3306/x", # MySQL
+ "http://example.com:6379/x", # Redis
+ "http://example.com:9200/x", # Elasticsearch
+ "http://example.com:5000/x", # a common internal admin port
+])
+def test_safe_url_refuses_non_web_ports(url, resolves_public):
+ with pytest.raises(UnsafeURL):
+ safe_url(url)
+
+
+@pytest.mark.parametrize("url", [
+ "http://example.com/x", # implicit 80
+ "https://example.com/x", # implicit 443
+ "http://example.com:80/x",
+ "https://example.com:443/x",
+ "http://example.com:8080/x",
+ "https://example.com:8443/x",
+])
+def test_safe_url_allows_the_web_ports(url, resolves_public):
+ assert safe_url(url) == url
+
+
def test_safe_url_accepts_a_public_host(resolves_public):
assert safe_url("https://example.com/some/page") == "https://example.com/some/page"
@@ -153,3 +177,19 @@ async def test_fetch_image_downscales(resolves_public):
assert jpeg and jpeg[:2] == b"\xff\xd8" # JPEG SOI
with Image.open(BytesIO(jpeg)) as im:
assert max(im.size) <= linkpreview._IMAGE_MAX_DIM
+
+
+async def test_fetch_image_refuses_a_decompression_bomb(resolves_public, monkeypatch):
+ from io import BytesIO
+
+ from PIL import Image
+ # A tiny file that reports enormous dimensions from its header alone.
+ monkeypatch.setattr(linkpreview, "_MAX_IMAGE_PIXELS", 1_000_000)
+ buf = BytesIO()
+ Image.new("RGB", (2000, 2000), (0, 0, 0)).save(buf, format="PNG") # 4 MP > cap
+ bomb = buf.getvalue()
+
+ def handler(request):
+ return httpx.Response(200, headers={"content-type": "image/png"}, content=bomb)
+ async with _client(handler) as c:
+ assert await linkpreview.fetch_image("https://example.com/x.png", client=c) is None