Skip to content

fix: close SSRF via redirect in /render and /screenshot - #61

Merged
Oaklight merged 1 commit into
masterfrom
fix/ssrf-redirect-guard
Oct 16, 2026
Merged

Oaklight merged 1 commit into
masterfrom
fix/ssrf-redirect-guard

Conversation

@Oaklight

Copy link
Copy Markdown
Owner

Summary

Closes an SSRF via redirect. The submitted URL was validated once, but the
browser then followed redirects without re-checking, and page.content() was
returned from wherever the navigation landed — so a public URL redirecting to an
internal address returned that address's response body to the caller. Affected
/render and /screenshot.

Reported privately; fixed and released together with the advisory.

Change

  • Per-request destination check. The Playwright route handler now re-checks
    every request's destination and aborts any host that does not resolve to a
    globally routable address. Because the redirect hop is a fresh request, this is
    what actually closes the redirect path — it also covers subresources.
  • Post-navigation re-validation. Both routes re-validate the URL the
    navigation actually landed on before returning or capturing anything.
  • is_global allowlist. _is_blocked_ip used an enumerated denylist of
    private ranges, which accepted CGNAT (100.64.0.0/10) and other
    special-purpose ranges. It now rejects anything not globally routable.
  • Handler always installed. browser.py only built the route handler when
    VEILRENDER_RESOURCE_FILTER was enabled and set it to None otherwise — so
    that setting skipped page.route() entirely and would have removed the new
    destination check along with ad/tracker filtering. An empty blocklist now
    disables filtering only.
  • SECURITY.md documenting private vulnerability reporting; CHANGELOG.md and
    version bump to 0.6.3.

Test plan

  • pytest tests/ -q — 81 passed, 4 failed. The 4 failures are in
    tests/test_fonts.py and are pre-existing: verified by stashing this change
    and re-running on a clean master, where the same 4 fail (4 failed, 5 passed).
    Unrelated to this change.
  • tests/test_url_validator.py + tests/test_filters.py (new, 43 tests) — pass.
    Cover non-public literals incl. CGNAT, localhost via the DNS path, the
    handler aborting a redirect to 127.0.0.1:7860/health, allowing public
    destinations and inline data: URLs, and the refusal cache.
  • ruff check, ruff format --check, ty check src/ — all pass.
  • End-to-end: reproduced the SSRF against a live deployment before the fix
    (public redirect → loopback returned the internal body), and will re-run the
    same request against 0.6.3 after deploy to confirm it is refused.

The submitted URL was validated once, but the browser then followed
redirects without re-checking, and page.content() was returned from
wherever the navigation landed. A public URL that redirected to an
internal address therefore had the internal response body returned to
the caller, unauthenticated SSRF with response exfiltration.

- Re-check the destination of every request in the Playwright route
  handler, aborting any host that does not resolve to a globally
  routable address. This covers the redirect hop, subresource fetches,
  and later navigations.
- Re-validate the URL a navigation actually landed on before returning
  content, in both /render and /screenshot.
- Replace the enumerated private-range denylist with an is_global
  allowlist, so CGNAT (100.64.0.0/10) and other special-purpose ranges
  are refused rather than fetched.
- Always install the route handler. It previously existed only when
  VEILRENDER_RESOURCE_FILTER was enabled and was otherwise None, so
  that setting would have silently removed the destination check
  along with ad/tracker filtering.
@Oaklight
Oaklight merged commit 7dfc9a8 into master Oct 16, 2026
2 checks passed
@Oaklight
Oaklight deleted the fix/ssrf-redirect-guard branch October 16, 2026 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant