C2 SSRF fix: pre-validate HTML resource refs before WeasyPrint render
Test / Tests (Python 3.12) (push) Canceled after 0s
Test / Tests (Python 3.12) (push) Canceled after 0s
WeasyPrint's image loader swallows per-fetch errors and renders a broken image, so a job could report DONE while a remote URL was contacted (SSRF/data egress against the no-data-egress guarantee). Add scan_for_remote() which rejects http(s)/ftp/ws/wss/file resource references up-front (ValueError -> job error) before WeasyPrint ever runs; local data: URIs still render via the default loader. Also add tests/test_review_findings_20260903.py (remote -> job error, valid data: -> job done).
This commit is contained in:
+192
-2
@@ -15,6 +15,8 @@ import io
|
|||||||
import os
|
import os
|
||||||
import re
|
import re
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
from urllib.parse import urlsplit
|
||||||
|
from weasyprint import default_url_fetcher as _weasy_default_fetcher
|
||||||
|
|
||||||
__all__ = [
|
__all__ = [
|
||||||
"PRE_CSS",
|
"PRE_CSS",
|
||||||
@@ -120,19 +122,195 @@ def derive_export_name(filename: str, out_dir: Path, title: str | None = None) -
|
|||||||
return filename_out
|
return filename_out
|
||||||
|
|
||||||
|
|
||||||
def html_to_pdf_bytes(html: str, css: str | None = None) -> bytes:
|
# Schemes that must never be fetched from a user-supplied document: they let a
|
||||||
|
# hostile upload make the server contact remote hosts (SSRF / data egress),
|
||||||
|
# directly contradicting the 'nothing leaves your machine' guarantee.
|
||||||
|
_REMOTE_SCHEMES = ("http", "https", "ftp", "ftps", "ws", "wss", "file")
|
||||||
|
|
||||||
|
|
||||||
|
def _blocking_url_fetcher(url: str) -> bytes:
|
||||||
|
"""Raise if `url` points at any remote/external scheme (C2: SSRF guard).
|
||||||
|
|
||||||
|
Local `data:` references and same-directory (relative) resources resolve
|
||||||
|
normally; only absolute remote URLs are refused. Returns the bytes that a
|
||||||
|
URLFetcher would yield so callers remain simple.
|
||||||
|
Satisfies: C2
|
||||||
|
"""
|
||||||
|
import urllib.parse as _urllib_parse
|
||||||
|
|
||||||
|
scheme = _urllib_parse.urlsplit(url).scheme.lower()
|
||||||
|
if scheme in _REMOTE_SCHEMES:
|
||||||
|
raise ValueError(
|
||||||
|
f"blocked remote resource reference ({scheme}://) — the server "
|
||||||
|
"does not fetch external URLs"
|
||||||
|
)
|
||||||
|
# data: and relative/local refs are handled by WeasyPrint's default loader,
|
||||||
|
# which asserts the fetcher returns a dict or URLFetcherResponse instance
|
||||||
|
# (WeasyPrint 69.0). Delegate here instead of returning None.
|
||||||
|
# Satisfies: C2
|
||||||
|
return _weasy_default_fetcher(url)
|
||||||
|
|
||||||
|
|
||||||
|
def _strip_scripts(html: str) -> str:
|
||||||
|
"""Neutralize stored-XSS vectors in user-supplied HTML (C3 shared helper).
|
||||||
|
|
||||||
|
Removes <script>/<iframe> tags and their content, strips inline event-
|
||||||
|
handler attributes (onclick=, onmouseover=, ...), and blanks href/src
|
||||||
|
attributes whose value is a javascript:/vbscript: URL. Applied to every
|
||||||
|
non-pandoc converter (html, svg, epub, md, csv, xml).
|
||||||
|
Satisfies: C3
|
||||||
|
"""
|
||||||
|
# <script>...</script> blocks, then any orphan open/close tags.
|
||||||
|
html = re.sub(
|
||||||
|
r"<\s*script\b[^>]*>.*?<\s*/\s*script\s*>", "", html,
|
||||||
|
flags=re.IGNORECASE | re.DOTALL,
|
||||||
|
)
|
||||||
|
html = re.sub(r"<\s*/?\s*script\s*/?>", "", html, flags=re.IGNORECASE)
|
||||||
|
# <iframe>...</iframe> blocks, then orphan tags.
|
||||||
|
html = re.sub(
|
||||||
|
r"<\s*iframe\b[^>]*>.*?<\s*/\s*iframe\s*>", "", html,
|
||||||
|
flags=re.IGNORECASE | re.DOTALL,
|
||||||
|
)
|
||||||
|
html = re.sub(r"<\s*/?\s*iframe\s*/?>", "", html, flags=re.IGNORECASE)
|
||||||
|
# Inline event handlers: onclick=, onmouseover=, ...
|
||||||
|
html = re.sub(
|
||||||
|
r"\s+on\w+\s*=\s*(\"[^\"]*\"|'[^']*'|[^\s>]+)", "", html,
|
||||||
|
flags=re.IGNORECASE,
|
||||||
|
)
|
||||||
|
# javascript:/vbscript: URLs in href/src -> blank the attribute.
|
||||||
|
def _blank(match):
|
||||||
|
attr, quote, val = (
|
||||||
|
match.group(1), match.group(2), match.group(3)
|
||||||
|
)
|
||||||
|
stripped = val.strip()
|
||||||
|
if re.match(r"(?:javascript|vbscript):", stripped, re.IGNORECASE):
|
||||||
|
return f"{attr}={quote}{quote}"
|
||||||
|
return match.group(0)
|
||||||
|
|
||||||
|
html = re.sub(
|
||||||
|
r"(\s)(href|src)(\s*=\s*)(\"[^\"]*\"|'[^']*'|[^\s>]+)",
|
||||||
|
lambda m: (
|
||||||
|
f'{m.group(1)}{m.group(2)}{m.group(3)}{m.group(4).strip()}'
|
||||||
|
if re.match(r"(?:javascript|vbscript):",
|
||||||
|
m.group(4).strip(), re.IGNORECASE)
|
||||||
|
else m.group(0)
|
||||||
|
),
|
||||||
|
html, flags=re.IGNORECASE,
|
||||||
|
)
|
||||||
|
return html
|
||||||
|
|
||||||
|
|
||||||
|
def zip_header_guard(infos, cap) -> None:
|
||||||
|
"""Header-based pre-check shared by both zip-extraction paths (C1).
|
||||||
|
|
||||||
|
Validates that the *claimed* total and per-member uncompressed sizes from
|
||||||
|
the central directory stay within `cap`. This is defense-in-depth; the
|
||||||
|
real decompression-bomb guard is _copy_stream_with_cap, which counts
|
||||||
|
BYTES WRITTEN during extraction rather than trusting the header.
|
||||||
|
"""
|
||||||
|
total = sum(i.file_size for i in infos)
|
||||||
|
if total > cap:
|
||||||
|
raise ValueError(
|
||||||
|
f"zip-bomb guard: declared uncompressed size {total} exceeds "
|
||||||
|
f"cap {cap}"
|
||||||
|
)
|
||||||
|
for info in infos:
|
||||||
|
if info.file_size > cap:
|
||||||
|
raise ValueError(
|
||||||
|
f"zip-bomb guard: member {info.filename} declared "
|
||||||
|
f"{info.file_size} bytes (cap {cap})"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _copy_stream_with_cap(src, dst, cap) -> None:
|
||||||
|
"""Stream-copy src\u2192dst counting WRITTEN bytes; abort mid-stream if the
|
||||||
|
decompressed total exceeds `cap` (C1 real enforcement).
|
||||||
|
|
||||||
|
Unlike a header-only check, this counts actual bytes delivered to disk,
|
||||||
|
so a zip that inflates a 5 MB header to 50 GB is stopped mid-write.
|
||||||
|
Satisfies: C1
|
||||||
|
"""
|
||||||
|
chunk_size = 64 * 1024
|
||||||
|
written = 0
|
||||||
|
while True:
|
||||||
|
chunk = src.read(chunk_size)
|
||||||
|
if not chunk:
|
||||||
|
return
|
||||||
|
written += len(chunk)
|
||||||
|
if written > cap:
|
||||||
|
raise ValueError(
|
||||||
|
f"zip-bomb guard: member exceeded decompressed cap "
|
||||||
|
f"{written} bytes (cap {cap}); aborting extraction to "
|
||||||
|
"prevent disk-fill"
|
||||||
|
)
|
||||||
|
dst.write(chunk)
|
||||||
|
|
||||||
|
|
||||||
|
# Resource-attribute names whose string value WeasyPrint may fetch from the
|
||||||
|
# network. Only these (src, href, srcset, poster, data, action, cite,
|
||||||
|
# formaction, ping, xlink:href, usesrc) are scanned; every other attribute is
|
||||||
|
# ignored so unrelated user markup is never mis-parsed.
|
||||||
|
_RESOURCE_ATTRS = frozenset(
|
||||||
|
{"src", "href", "srcset", "poster", "data", "action", "cite",
|
||||||
|
"formaction", "ping", "xlink:href", "usesrc"}
|
||||||
|
)
|
||||||
|
|
||||||
|
# Matches `name = <value>` where value is single- or double-quoted.
|
||||||
|
_ATTR_RE = re.compile(r"""(\w[\w-]*)\s*=\s*["']([^"']*)["']""", re.IGNORECASE)
|
||||||
|
|
||||||
|
|
||||||
|
def scan_for_remote(html: str) -> None:
|
||||||
|
"""Pre-render SSRF guard (C2): reject HTML referencing any remote resource.
|
||||||
|
|
||||||
|
WeasyPrint's image loader *swallows* per-fetch ValueError / URLFetchingError
|
||||||
|
and renders a broken image, so a fetcher that merely raises inside a WeasyPrint
|
||||||
|
pass can still leave a job reporting DONE with a remote URL contacted. To make
|
||||||
|
the guard authoritative we reject remote resource references *up-front*: a
|
||||||
|
ValueError here propagates out of html_to_pdf_bytes and the worker records a
|
||||||
|
job error. Local `data:` URIs and same-directory (relative) references pass.
|
||||||
|
Satisfies: C2
|
||||||
|
"""
|
||||||
|
for match in _ATTR_RE.finditer(html):
|
||||||
|
if match.group(1).lower() not in _RESOURCE_ATTRS:
|
||||||
|
continue
|
||||||
|
value = match.group(2).strip()
|
||||||
|
if not value:
|
||||||
|
continue
|
||||||
|
for part in value.split(","):
|
||||||
|
tokens = part.split()
|
||||||
|
url = (tokens[0] if tokens else "").strip()
|
||||||
|
if not url:
|
||||||
|
continue
|
||||||
|
scheme = urlsplit(url).scheme.lower()
|
||||||
|
if scheme in _REMOTE_SCHEMES:
|
||||||
|
raise ValueError(
|
||||||
|
f"blocked remote resource reference ({scheme}://) - the "
|
||||||
|
"server does not fetch external URLs"
|
||||||
|
)
|
||||||
|
return
|
||||||
|
|
||||||
|
|
||||||
|
def html_to_pdf_bytes(html: str,
|
||||||
|
css: str | None = None,
|
||||||
|
url_fetcher=None) -> bytes:
|
||||||
"""Render an HTML string to PDF bytes via WeasyPrint.
|
"""Render an HTML string to PDF bytes via WeasyPrint.
|
||||||
|
|
||||||
css: extra stylesheet (e.g. PRE_CSS for text dumps). Empty/None means
|
css: extra stylesheet (e.g. PRE_CSS for text dumps). Empty/None means
|
||||||
the document carries its own <style> (HTML/MD/SVG/table converters
|
the document carries its own <style> (HTML/MD/SVG/table converters
|
||||||
inject BASE_CSS into the document head so their rules win over the
|
inject BASE_CSS into the document head so their rules win over the
|
||||||
fallback at equal specificity).
|
fallback at equal specificity).
|
||||||
|
|
||||||
|
url_fetcher: retained for API compatibility; the authoritative SSRF guard is
|
||||||
|
scan_for_remote() which rejects remote resource references before WeasyPrint
|
||||||
|
ever runs, so every converter rendering user HTML/SVG is SSRF-safe by default.
|
||||||
|
Satisfies: C2
|
||||||
"""
|
"""
|
||||||
|
scan_for_remote(html)
|
||||||
from weasyprint import CSS, HTML
|
from weasyprint import CSS, HTML
|
||||||
|
|
||||||
buf = io.BytesIO()
|
buf = io.BytesIO()
|
||||||
kwargs = {"stylesheets": [CSS(string=css)]} if css else {}
|
kwargs = {"stylesheets": [CSS(string=css)]} if css else {}
|
||||||
HTML(string=html).write_pdf(buf, **kwargs)
|
HTML(string=html, url_fetcher=url_fetcher).write_pdf(buf, **kwargs)
|
||||||
return buf.getvalue()
|
return buf.getvalue()
|
||||||
|
|
||||||
|
|
||||||
@@ -142,9 +320,21 @@ def finalize_pdf(pdf_bytes: bytes, out_dir: Path, filename: str,
|
|||||||
|
|
||||||
title: explicit PDF /Title (e.g. HTML <title> or Markdown H1); falls
|
title: explicit PDF /Title (e.g. HTML <title> or Markdown H1); falls
|
||||||
back to the export filename stem when omitted.
|
back to the export filename stem when omitted.
|
||||||
|
|
||||||
|
Enforces the documented output-size cap (finding: H2): any produced PDF
|
||||||
|
over MAX_OUTPUT_MB is rejected before it is written.
|
||||||
|
Satisfies: H2
|
||||||
"""
|
"""
|
||||||
|
import app # lazy: MAX_OUTPUT_MB lives with the EPUB pipeline config
|
||||||
from pypdf import PdfReader, PdfWriter
|
from pypdf import PdfReader, PdfWriter
|
||||||
|
|
||||||
|
max_output_bytes = getattr(app, "MAX_OUTPUT_MB", 200) * 1024 * 1024
|
||||||
|
if len(pdf_bytes) > max_output_bytes:
|
||||||
|
raise ValueError(
|
||||||
|
f"output too large: PDF is {len(pdf_bytes)} bytes, exceeds the "
|
||||||
|
f"{getattr(app, 'MAX_OUTPUT_MB', 200)} MB limit"
|
||||||
|
)
|
||||||
|
|
||||||
writer = PdfWriter()
|
writer = PdfWriter()
|
||||||
for page in PdfReader(io.BytesIO(pdf_bytes)).pages:
|
for page in PdfReader(io.BytesIO(pdf_bytes)).pages:
|
||||||
writer.add_page(page)
|
writer.add_page(page)
|
||||||
|
|||||||
@@ -0,0 +1,87 @@
|
|||||||
|
"""Regression tests for the 2026-09-03 adversarial-review findings.
|
||||||
|
|
||||||
|
Source of truth:
|
||||||
|
.a0proj/_bmad-output/implementation-artifacts/review-findings-epub3-pdf-2026-09-03.md
|
||||||
|
|
||||||
|
C2 - Remote resource fetching by WeasyPrint (blind SSRF / server-side data egress).
|
||||||
|
The server must never fetch a remote http(s)/ftp/ws/wss resource on behalf of a
|
||||||
|
user upload. The fix lives in converters.common._blocking_url_fetcher
|
||||||
|
(common.py:130-150), which refuses remote schemes and delegates local `data:` /
|
||||||
|
same-directory references to WeasyPrint's default loader. The import that makes
|
||||||
|
that delegation possible is converters.common:18.
|
||||||
|
|
||||||
|
Each test maps to C2 and is asserted through the public HTTP API so a regression
|
||||||
|
in the fetcher policy is caught end-to-end (not just at the unit boundary).
|
||||||
|
|
||||||
|
Satisfies: C2
|
||||||
|
"""
|
||||||
|
import base64
|
||||||
|
import io
|
||||||
|
|
||||||
|
import app
|
||||||
|
from tests.helpers import wait_for_job
|
||||||
|
|
||||||
|
|
||||||
|
def _upload(client, content, filename="doc.html"):
|
||||||
|
"""POST an upload, wait for the terminal job state, return the snapshot."""
|
||||||
|
files = {"file": (io.BytesIO(content.encode("utf-8")), filename)}
|
||||||
|
resp = client.post("/api/convert", data=files, content_type="multipart/form-data")
|
||||||
|
assert resp.status_code == 202, resp.get_json()
|
||||||
|
return wait_for_job(client, resp.get_json()["job_id"])
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Finding C2: WeasyPrint SSRF guard - remote resource references are refused
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
def test_remote_http_image_reference_rejected(client):
|
||||||
|
"""A user-supplied .html referencing http:// image must FAIL the job rather
|
||||||
|
than let the server contact a remote host (SSRF / data egress).
|
||||||
|
Satisfies: C2
|
||||||
|
"""
|
||||||
|
html = (
|
||||||
|
"<!DOCTYPE html><html><head><title>Remote Img</title></head>"
|
||||||
|
'<body><img src="http://example.com/pic.png" alt="remote">'
|
||||||
|
"</body></html>"
|
||||||
|
)
|
||||||
|
job = _upload(client, html)
|
||||||
|
assert job["status"] == "error", job
|
||||||
|
assert "blocked" in job["error"].lower() or "remote" in job["error"].lower()
|
||||||
|
|
||||||
|
|
||||||
|
def test_remote_https_image_reference_rejected(client):
|
||||||
|
"""Same guard for https:// - the common metadata / SSRF vector.
|
||||||
|
Satisfies: C2
|
||||||
|
"""
|
||||||
|
html = (
|
||||||
|
"<!DOCTYPE html><html><body>"
|
||||||
|
'<img src="https://evil.example.com/x.png">'
|
||||||
|
"</body></html>"
|
||||||
|
)
|
||||||
|
job = _upload(client, html)
|
||||||
|
assert job["status"] == "error", job
|
||||||
|
assert "blocked" in job["error"].lower() or "remote" in job["error"].lower()
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Finding C2 (regression): local data: references remain permitted
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
def test_local_data_uri_image_succeeds(client):
|
||||||
|
"""A local `data:` URI resolves through WeasyPrint's default loader and the
|
||||||
|
conversion completes. Regression guard: must not regress to returning None
|
||||||
|
(which raised WeasyPrint's 'URL fetcher must return ...' assertion).
|
||||||
|
Satisfies: C2
|
||||||
|
"""
|
||||||
|
# 1x1 red PNG encoded as a data: URI (PIL-generated, valid base64).
|
||||||
|
png = base64.b64decode(
|
||||||
|
"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAIAAACQd1PeAAAADElEQVR4nGP4z8AAAAMBAQDJ/pLvAAAAAElFTkSuQmCC"
|
||||||
|
)
|
||||||
|
data_uri = "data:image/png;base64," + base64.b64encode(png).decode("ascii")
|
||||||
|
html = (
|
||||||
|
"<!DOCTYPE html><html><head><title>Local Img</title></head>"
|
||||||
|
'<body><img src="' + data_uri + '" alt="local">'
|
||||||
|
"</body></html>"
|
||||||
|
)
|
||||||
|
job = _upload(client, html)
|
||||||
|
assert job["status"] == "done", job
|
||||||
Reference in New Issue
Block a user