Skip to main content

max / makenotwork

mt: proxy external Fan+ images, static template-500 fallback (ultra-fuzz M-UX1 + UX NOTE) M-UX1: CSP img-src 'self' silently blocked the external images the Fan+ markdown renderer emits. Rather than widen the policy, route them through a same-origin proxy so the CSP stays tight. The renderer rewrites external <img src> to /img-proxy?u=...; image_proxy_handler is login-gated and in the per-IP image rate-limit group, fetches via link_preview's SSRF-safe client (private addresses refused at connect time, even across redirects), caps 1 MB / 5 s, and re-serves only an image content-type allowlist (png/jpeg/gif/webp). Needs a browser check before launch (paid feature). Template-render failures returned a raw "Template error" 500; now a fully static branded page (error_page::internal_error_static) that can't re-enter the failing template engine. ?toast= code-map left as-is by decision (already XSS-safe; low-value spoof). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-06-23 13:57 UTC
Commit: b0bf5fc85de3bfee2b599889b846d5ca96f2d60f
Parent: 2290bc6
7 files changed, +251 insertions, -8 deletions
M multithreaded/src/error_page.rs +24
@@ -59,3 +59,27 @@ pub fn internal_error() -> Response {
59 59 )
60 60 .into_response()
61 61 }
62 +
63 + /// A fully static, template-free branded 500. Used only when the Askama render
64 + /// itself fails — falling back to [`internal_error`] there would just re-enter
65 + /// the template engine that's already broken. No interpolation, so it can never
66 + /// fail to produce output.
67 + pub fn internal_error_static() -> Response {
68 + const BODY: &str = "<!DOCTYPE html>\n\
69 + <html lang=\"en\"><head><meta charset=\"utf-8\">\
70 + <meta name=\"viewport\" content=\"width=device-width, initial-scale=1\">\
71 + <title>Something went wrong</title>\
72 + <style>body{font-family:system-ui,-apple-system,sans-serif;background:#111;color:#eee;\
73 + margin:0;min-height:100vh;display:flex;align-items:center;justify-content:center}\
74 + main{max-width:32rem;padding:2rem;text-align:center}.mark{font-size:2rem;opacity:.55}\
75 + h1{font-weight:600;font-size:1.25rem;margin:.75rem 0}a{color:#9ad}</style></head>\
76 + <body><main><div class=\"mark\">&#9670;</div>\
77 + <h1>Something went wrong</h1>\
78 + <p>This page couldn't be rendered. The error has been logged. Try again, or return to the <a href=\"/\">forum home</a>.</p>\
79 + </main></body></html>";
80 + (
81 + StatusCode::INTERNAL_SERVER_ERROR,
82 + axum::response::Html(BODY),
83 + )
84 + .into_response()
85 + }
M multithreaded/src/link_preview.rs +76
@@ -234,6 +234,70 @@ pub async fn fetch_og_metadata(
234 234 }
235 235 }
236 236
237 + /// Normalize an external image response's `Content-Type` to one of the four
238 + /// formats we re-serve, or `None` to reject (the proxy default-denies anything
239 + /// that isn't a recognised image type, so it can't be turned into a relay for
240 + /// arbitrary content).
241 + fn allowed_image_content_type(ct: &str) -> Option<&'static str> {
242 + match ct.split(';').next().unwrap_or("").trim() {
243 + "image/png" => Some("image/png"),
244 + "image/jpeg" => Some("image/jpeg"),
245 + "image/gif" => Some("image/gif"),
246 + "image/webp" => Some("image/webp"),
247 + _ => None,
248 + }
249 + }
250 +
251 + /// Fetch an external image for the same-origin image proxy.
252 + ///
253 + /// Same SSRF guarantees as [`fetch_og_metadata`] (scheme/port pre-check +
254 + /// connect-time `SsrfSafeResolver` that refuses private addresses, even across
255 + /// redirects), capped at `MAX_BODY_SIZE` and a 5s timeout. Returns the bytes and
256 + /// a normalised image content-type, or `None` on any failure or a non-image
257 + /// response. Best-effort by design: the caller maps `None` to a 502.
258 + #[tracing::instrument(skip_all)]
259 + pub async fn fetch_image(http: &reqwest::Client, url: &str) -> Option<(Vec<u8>, &'static str)> {
260 + if !validate_url(url) {
261 + return None;
262 + }
263 +
264 + let resp = http
265 + .get(url)
266 + .timeout(std::time::Duration::from_secs(5))
267 + .header("User-Agent", "Multithreaded/ImageProxy")
268 + .send()
269 + .await
270 + .ok()?;
271 +
272 + if !resp.status().is_success() {
273 + return None;
274 + }
275 +
276 + // Default-deny: only re-serve a recognised image content-type.
277 + let content_type = resp
278 + .headers()
279 + .get(CONTENT_TYPE)
280 + .and_then(|ct| ct.to_str().ok())
281 + .and_then(allowed_image_content_type)?;
282 +
283 + // Read the body in chunks, hard-capped at MAX_BODY_SIZE (1 MB).
284 + let mut body = Vec::new();
285 + let mut stream = resp;
286 + while body.len() < MAX_BODY_SIZE {
287 + let chunk = match stream.chunk().await.ok()? {
288 + Some(c) => c,
289 + None => break,
290 + };
291 + let remaining = MAX_BODY_SIZE - body.len();
292 + body.extend_from_slice(&chunk[..chunk.len().min(remaining)]);
293 + }
294 +
295 + if body.is_empty() {
296 + return None;
297 + }
298 + Some((body, content_type))
299 + }
300 +
237 301 /// Extract a `<meta property="..." content="...">` value from HTML.
238 302 fn extract_og_meta(html: &str, property: &str) -> Option<String> {
239 303 static OG_RE: std::sync::LazyLock<regex_lite::Regex> = std::sync::LazyLock::new(|| {
@@ -289,6 +353,18 @@ mod tests {
289 353 }
290 354
291 355 #[test]
356 + fn image_proxy_content_type_allowlist() {
357 + // Recognised image types pass (with parameters stripped); everything else
358 + // is refused so the proxy can't re-serve HTML/SVG/arbitrary content.
359 + assert_eq!(allowed_image_content_type("image/png"), Some("image/png"));
360 + assert_eq!(allowed_image_content_type("image/jpeg; charset=binary"), Some("image/jpeg"));
361 + assert_eq!(allowed_image_content_type("image/webp"), Some("image/webp"));
362 + assert_eq!(allowed_image_content_type("image/svg+xml"), None);
363 + assert_eq!(allowed_image_content_type("text/html"), None);
364 + assert_eq!(allowed_image_content_type(""), None);
365 + }
366 +
367 + #[test]
292 368 fn extract_urls_skips_non_http() {
293 369 let input = "[mail](mailto:a@b.com) [site](https://x.com)";
294 370 let urls = extract_urls(input);
M multithreaded/src/routes/helpers.rs +74 -2
@@ -38,9 +38,44 @@ pub(crate) fn render_markdown(input: &str) -> String {
38 38 /// the strict preset — raw HTML is still stripped, dangerous schemes filtered,
39 39 /// links get `nofollow`. Use for Fan+ subscriber content.
40 40 pub(crate) fn render_markdown_plus(input: &str) -> String {
41 - docengine::Renderer::strict()
41 + let html = docengine::Renderer::strict()
42 42 .with_strip_images(false)
43 - .render(input)
43 + .render(input);
44 + proxy_external_images(&html)
45 + }
46 +
47 + /// Rewrite external `<img src="http(s)://…">` in rendered Fan+ HTML to load
48 + /// through the same-origin image proxy (`/img-proxy?u=…`), so the page CSP can
49 + /// stay `img-src 'self'` while still showing embedded external images. Uploaded
50 + /// images are served from same-origin `/uploads/…` (no scheme), so they don't
51 + /// match and aren't proxied. docengine has already attribute-escaped the URL, so
52 + /// `&amp;` in a query string is unescaped back to `&` before percent-encoding.
53 + fn proxy_external_images(html: &str) -> String {
54 + static IMG_SRC_RE: std::sync::LazyLock<regex_lite::Regex> = std::sync::LazyLock::new(|| {
55 + regex_lite::Regex::new(r#"(<img\b[^>]*?\bsrc=")(https?://[^"]+)(")"#).unwrap()
56 + });
57 + IMG_SRC_RE
58 + .replace_all(html, |caps: &regex_lite::Captures| {
59 + let url = caps[2].replace("&amp;", "&");
60 + format!("{}/img-proxy?u={}{}", &caps[1], percent_encode_query(&url), &caps[3])
61 + })
62 + .into_owned()
63 + }
64 +
65 + /// RFC 3986 percent-encode a string for use as a URL query value. Keeps the
66 + /// unreserved set (`A-Za-z0-9-_.~`) and escapes everything else, so the proxied
67 + /// URL round-trips intact through axum's query decoder.
68 + fn percent_encode_query(s: &str) -> String {
69 + let mut out = String::with_capacity(s.len());
70 + for &b in s.as_bytes() {
71 + match b {
72 + b'A'..=b'Z' | b'a'..=b'z' | b'0'..=b'9' | b'-' | b'_' | b'.' | b'~' => {
73 + out.push(b as char)
74 + }
75 + _ => out.push_str(&format!("%{b:02X}")),
76 + }
77 + }
78 + out
44 79 }
45 80
46 81 /// Render markdown to HTML, resolving `@mentions` to profile links for valid community members.
@@ -515,6 +550,43 @@ pub(crate) fn check_community_state(
515 550 }
516 551
517 552 #[cfg(test)]
553 + mod proxy_image_tests {
554 + use super::{percent_encode_query, proxy_external_images};
555 +
556 + #[test]
557 + fn external_image_is_routed_through_proxy() {
558 + let html = r#"<p><img src="https://example.com/a.png" alt="x"></p>"#;
559 + let out = proxy_external_images(html);
560 + assert!(
561 + out.contains(r#"src="/img-proxy?u=https%3A%2F%2Fexample.com%2Fa.png""#),
562 + "got: {out}"
563 + );
564 + }
565 +
566 + #[test]
567 + fn same_origin_upload_is_not_proxied() {
568 + let html = r#"<img src="/uploads/abc-123" alt="x">"#;
569 + assert_eq!(proxy_external_images(html), html, "relative uploads must be left alone");
570 + }
571 +
572 + #[test]
573 + fn query_string_ampersands_round_trip() {
574 + // docengine attribute-escapes `&` to `&amp;`; the proxied value must carry
575 + // the real `&` (percent-encoded), not the entity.
576 + let html = r#"<img src="https://x.test/i?w=1&amp;h=2">"#;
577 + let out = proxy_external_images(html);
578 + assert!(out.contains("u=https%3A%2F%2Fx.test%2Fi%3Fw%3D1%26h%3D2"), "got: {out}");
579 + assert!(!out.contains("&amp;h=2"), "the entity must not leak into the query");
580 + }
581 +
582 + #[test]
583 + fn percent_encode_keeps_unreserved() {
584 + assert_eq!(percent_encode_query("aZ0-_.~"), "aZ0-_.~");
585 + assert_eq!(percent_encode_query("a/b?c=d&e"), "a%2Fb%3Fc%3Dd%26e");
586 + }
587 + }
588 +
589 + #[cfg(test)]
518 590 mod validation_tests {
519 591 use super::*;
520 592
M multithreaded/src/routes/mod.rs +1
@@ -170,6 +170,7 @@ pub fn forum_routes(state: AppState) -> Router {
170 170
171 171 let image_routes = Router::new()
172 172 .route("/uploads/{id}", get(uploads::serve_image_handler))
173 + .route("/img-proxy", get(uploads::image_proxy_handler))
173 174 .route_layer(GovernorLayer {
174 175 config: image_rate_limit.clone(),
175 176 });
M multithreaded/src/routes/uploads.rs +54 -1
@@ -1,10 +1,13 @@
1 1 //! Image upload and serving handlers.
2 2
3 3 use axum::{
4 - extract::{Multipart, Path},
4 + body::Body,
5 + extract::{Multipart, Path, Query},
5 6 http::{StatusCode, header},
6 7 response::{IntoResponse, Response},
7 8 };
9 + use serde::Deserialize;
10 +
8 11 use mt_core::types::{ModAction, ModActor};
9 12
10 13 use crate::auth::MaybeUser;
@@ -270,3 +273,53 @@ pub(super) async fn remove_image_handler(
270 273
271 274 Ok(StatusCode::OK)
272 275 }
276 +
277 + /// Query for [`image_proxy_handler`].
278 + #[derive(Deserialize)]
279 + pub(super) struct ImageProxyQuery {
280 + /// The external image URL to fetch (percent-encoded by the renderer).
281 + u: String,
282 + }
283 +
284 + /// GET /img-proxy?u=<url> — same-origin proxy for external Fan+ images.
285 + ///
286 + /// The Fan+ markdown renderer rewrites external `<img src="https://…">` to point
287 + /// here so the page CSP can stay `img-src 'self'` (M-UX1). This handler fetches
288 + /// the URL through the SSRF-safe link-preview client (private addresses refused
289 + /// at connect time, even across redirects), caps the body at 1 MB / 5 s, and
290 + /// re-serves only recognised image content-types.
291 + ///
292 + /// Login-gated so it can't be driven as an open image-fetch relay by anonymous
293 + /// clients, and it sits in the per-IP image rate-limit group.
294 + #[tracing::instrument(skip_all)]
295 + pub(super) async fn image_proxy_handler(
296 + axum::extract::State(state): axum::extract::State<AppState>,
297 + MaybeUser(session_user): MaybeUser,
298 + Query(query): Query<ImageProxyQuery>,
299 + ) -> Result<Response, Response> {
300 + // Authenticated members only — an anonymous open proxy is the abuse vector.
301 + let _user = session_user.ok_or_else(|| StatusCode::UNAUTHORIZED.into_response())?;
302 +
303 + let client = match &state.link_preview {
304 + crate::link_preview::LinkPreviewFetcher::Http(c) => c,
305 + crate::link_preview::LinkPreviewFetcher::Noop => {
306 + return Err(StatusCode::SERVICE_UNAVAILABLE.into_response());
307 + }
308 + };
309 +
310 + let (bytes, content_type) = crate::link_preview::fetch_image(client, &query.u)
311 + .await
312 + .ok_or_else(|| StatusCode::BAD_GATEWAY.into_response())?;
313 +
314 + // `nosniff` pins the browser to the validated image content-type, so even if
315 + // an upstream served image bytes under a benign type, it can't be reframed as
316 + // HTML/JS. 1 MB cap means buffering here is bounded.
317 + Ok(Response::builder()
318 + .status(StatusCode::OK)
319 + .header(header::CONTENT_TYPE, content_type)
320 + .header(header::CACHE_CONTROL, "private, max-age=86400")
321 + .header(header::X_CONTENT_TYPE_OPTIONS, "nosniff")
322 + .header(header::CONTENT_DISPOSITION, "inline")
323 + .body(Body::from(bytes))
324 + .unwrap())
325 + }
M multithreaded/src/templates/mod.rs +4 -5
@@ -10,10 +10,7 @@ mod partials;
10 10 pub use public::*;
11 11
12 12 use askama::Template;
13 - use axum::{
14 - http::StatusCode,
15 - response::{Html, IntoResponse, Response},
16 - };
13 + use axum::response::{Html, IntoResponse, Response};
17 14
18 15 /// CSRF token passed to templates; None when no session store is configured.
19 16 pub type CsrfTokenOption = Option<String>;
@@ -24,7 +21,9 @@ fn render_template<T: Template>(template: T) -> Response {
24 21 Ok(html) => Html(html).into_response(),
25 22 Err(err) => {
26 23 tracing::error!(error = ?err, "template rendering error");
27 - (StatusCode::INTERNAL_SERVER_ERROR, "Template error").into_response()
24 + // Static branded page, not the Error500 *template* — the engine that
25 + // just failed can't be trusted to render the fallback.
26 + crate::error_page::internal_error_static()
28 27 }
29 28 }
30 29 }
M multithreaded/tests/workflows/uploads.rs +18
@@ -36,3 +36,21 @@ async fn serve_image_invalid_uuid_returns_404() {
36 36 let resp = h.client.get("/uploads/not-a-uuid").await;
37 37 assert_eq!(resp.status.as_u16(), 404, "Invalid UUID should return 404");
38 38 }
39 +
40 + /// The external-image proxy is login-gated: an anonymous request is rejected
41 + /// before any outbound fetch, so it can't be driven as an open relay (M-UX1).
42 + #[tokio::test]
43 + async fn image_proxy_requires_login() {
44 + let mut h = TestHarness::new().await;
45 + h.client.get("/").await; // establish a session, but stay logged out
46 +
47 + let resp = h
48 + .client
49 + .get("/img-proxy?u=https%3A%2F%2Fexample.com%2Fa.png")
50 + .await;
51 + assert_eq!(
52 + resp.status.as_u16(),
53 + 401,
54 + "anonymous image-proxy request must be rejected before any fetch"
55 + );
56 + }