Skip to content

Commit 56f3821

Browse files
rtrompierrtrompier
authored
fix: prevent SSRF via open redirect bypass in fetch-url endpoint (#2119)
1 parent 9b03a64 commit 56f3821

File tree

2 files changed

+165
-63
lines changed

2 files changed

+165
-63
lines changed

‎src/lib/server/urlSafety.ts‎

Lines changed: 54 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,68 @@
1-
// Shared server-side URL safety helper (exact behavior preserved)
1+
import { Address4, Address6 } from "ip-address";
2+
import { isIP } from "node:net";
3+
4+
const UNSAFE_IPV4_SUBNETS = [
5+
"0.0.0.0/8",
6+
"10.0.0.0/8",
7+
"100.64.0.0/10",
8+
"127.0.0.0/8",
9+
"169.254.0.0/16",
10+
"172.16.0.0/12",
11+
"192.168.0.0/16",
12+
].map((s) => new Address4(s));
13+
14+
function isUnsafeIp(address: string): boolean {
15+
const family = isIP(address);
16+
17+
if (family === 4) {
18+
const addr = new Address4(address);
19+
return UNSAFE_IPV4_SUBNETS.some((subnet) => addr.isInSubnet(subnet));
20+
}
21+
22+
if (family === 6) {
23+
const addr = new Address6(address);
24+
// Check IPv4-mapped IPv6 addresses (e.g. ::ffff:127.0.0.1)
25+
if (addr.is4()) {
26+
const v4 = addr.to4();
27+
return UNSAFE_IPV4_SUBNETS.some((subnet) => v4.isInSubnet(subnet));
28+
}
29+
return addr.isLoopback() || addr.isLinkLocal();
30+
}
31+
32+
return true; // Unknown format → block
33+
}
34+
35+
/**
36+
* Synchronous URL validation: checks protocol and hostname string.
37+
*/
238
export function isValidUrl(urlString: string): boolean {
339
try {
440
const url = new URL(urlString.trim());
5-
// Only allow HTTPS protocol
641
if (url.protocol !== "https:") {
742
return false;
843
}
9-
// Prevent localhost/private IPs (basic check)
1044
const hostname = url.hostname.toLowerCase();
11-
if (
12-
hostname === "localhost" ||
13-
hostname.startsWith("127.") ||
14-
hostname.startsWith("192.168.") ||
15-
hostname.startsWith("172.16.") ||
16-
hostname === "[::1]" ||
17-
hostname === "0.0.0.0"
18-
) {
45+
if (hostname === "localhost") {
1946
return false;
2047
}
48+
// If the hostname is a raw IP literal, validate it
49+
const cleanHostname = hostname.replace(/^\[|]$/g, "");
50+
if (isIP(cleanHostname)) {
51+
return !isUnsafeIp(cleanHostname);
52+
}
2153
return true;
2254
} catch {
2355
return false;
2456
}
2557
}
58+
59+
/**
60+
* Assert that a resolved IP address is safe (not internal/private).
61+
* Throws if the IP is internal. Used in undici's custom DNS lookup
62+
* to validate IPs at connection time (prevents TOCTOU DNS rebinding).
63+
*/
64+
export function assertSafeIp(address: string, hostname: string): void {
65+
if (isUnsafeIp(address)) {
66+
throw new Error(`Resolved IP for ${hostname} is internal (${address})`);
67+
}
68+
}

‎src/routes/api/fetch-url/+server.ts‎

Lines changed: 111 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
import { error } from "@sveltejs/kit";
22
import { logger } from "$lib/server/logger.js";
3-
import { fetch } from "undici";
4-
import { isValidUrl } from "$lib/server/urlSafety";
3+
import { Agent, fetch } from "undici";
4+
import { isValidUrl, assertSafeIp } from "$lib/server/urlSafety";
5+
import dns from "node:dns";
56

67
const MAX_FILE_SIZE = 10 * 1024 * 1024; // 10MB
78
const FETCH_TIMEOUT = 30000; // 30 seconds
9+
const MAX_REDIRECTS = 5;
810
const SECURITY_HEADERS: HeadersInit = {
911
// Prevent any active content from executing if someone navigates directly to this endpoint.
1012
"Content-Security-Policy":
@@ -14,6 +16,36 @@ const SECURITY_HEADERS: HeadersInit = {
1416
"Referrer-Policy": "no-referrer",
1517
};
1618

19+
/**
20+
* Undici dispatcher that validates resolved IPs at connection time,
21+
* preventing TOCTOU DNS rebinding attacks.
22+
*/
23+
const ssrfSafeAgent = new Agent({
24+
connect: {
25+
lookup: (hostname, options, callback) => {
26+
dns.lookup(hostname, options, (err, address, family) => {
27+
if (err) return callback(err, "", 4);
28+
if (typeof address === "string") {
29+
try {
30+
assertSafeIp(address, hostname);
31+
} catch (e) {
32+
return callback(e as Error, "", 4);
33+
}
34+
} else if (Array.isArray(address)) {
35+
for (const entry of address) {
36+
try {
37+
assertSafeIp(entry.address, hostname);
38+
} catch (e) {
39+
return callback(e as Error, "", 4);
40+
}
41+
}
42+
}
43+
return callback(null, address, family);
44+
});
45+
},
46+
},
47+
});
48+
1749
export async function GET({ url }) {
1850
const targetUrl = url.searchParams.get("url");
1951

@@ -27,62 +59,89 @@ export async function GET({ url }) {
2759
throw error(400, "Invalid or unsafe URL (only HTTPS is supported)");
2860
}
2961

62+
// Fetch with timeout, following redirects manually to validate each hop
63+
const controller = new AbortController();
64+
const timeoutId = setTimeout(() => controller.abort(), FETCH_TIMEOUT);
65+
66+
let currentUrl = targetUrl;
67+
let response: Awaited<ReturnType<typeof fetch>>;
68+
let redirectCount = 0;
69+
3070
try {
31-
// Fetch with timeout
32-
const controller = new AbortController();
33-
const timeoutId = setTimeout(() => controller.abort(), FETCH_TIMEOUT);
34-
35-
const response = await fetch(targetUrl, {
36-
signal: controller.signal,
37-
headers: {
38-
"User-Agent": "HuggingChat-Attachment-Fetcher/1.0",
39-
},
40-
}).finally(() => clearTimeout(timeoutId));
41-
42-
if (!response.ok) {
43-
logger.error({ targetUrl, response }, "Error fetching URL. Response not ok.");
44-
throw error(response.status, `Failed to fetch: ${response.statusText}`);
45-
}
71+
// eslint-disable-next-line no-constant-condition
72+
while (true) {
73+
response = await fetch(currentUrl, {
74+
signal: controller.signal,
75+
redirect: "manual",
76+
dispatcher: ssrfSafeAgent,
77+
headers: {
78+
"User-Agent": "HuggingChat-Attachment-Fetcher/1.0",
79+
},
80+
});
4681

47-
// Check content length if available
48-
const contentLength = response.headers.get("content-length");
49-
if (contentLength && parseInt(contentLength) > MAX_FILE_SIZE) {
50-
throw error(413, "File too large (max 10MB)");
51-
}
82+
if (response.status >= 300 && response.status < 400) {
83+
redirectCount++;
84+
if (redirectCount > MAX_REDIRECTS) {
85+
throw error(502, "Too many redirects");
86+
}
5287

53-
// Stream the response back
54-
const originalContentType = response.headers.get("content-type") || "application/octet-stream";
55-
// Send as text/plain for safety; expose the original type via secondary header
56-
const safeContentType = "text/plain; charset=utf-8";
57-
const contentDisposition = response.headers.get("content-disposition");
58-
59-
const headers: HeadersInit = {
60-
"Content-Type": safeContentType,
61-
"X-Forwarded-Content-Type": originalContentType,
62-
"Cache-Control": "public, max-age=3600",
63-
...(contentDisposition ? { "Content-Disposition": contentDisposition } : {}),
64-
...SECURITY_HEADERS,
65-
};
66-
67-
// Get the body as array buffer to check size
68-
const arrayBuffer = await response.arrayBuffer();
69-
70-
if (arrayBuffer.byteLength > MAX_FILE_SIZE) {
71-
throw error(413, "File too large (max 10MB)");
72-
}
88+
const location = response.headers.get("location");
89+
if (!location) {
90+
throw error(502, "Redirect without Location header");
91+
}
92+
93+
// Resolve relative redirects against the current URL
94+
const redirectUrl = new URL(location, currentUrl).toString();
7395

74-
return new Response(arrayBuffer, { headers });
75-
} catch (err) {
76-
if (err instanceof Error) {
77-
if (err.name === "AbortError") {
78-
logger.error(err, "Request timeout");
79-
throw error(504, "Request timeout");
96+
if (!isValidUrl(redirectUrl)) {
97+
logger.warn(
98+
{ redirectUrl, originalUrl: targetUrl },
99+
"Redirect to unsafe URL blocked (SSRF)"
100+
);
101+
throw error(403, "Redirect target is not allowed");
102+
}
103+
104+
currentUrl = redirectUrl;
105+
continue;
80106
}
81107

82-
logger.error(err, "Error fetching URL");
83-
throw error(500, `Failed to fetch URL: ${err.message}`);
108+
break;
84109
}
85-
logger.error(err, "Error fetching URL");
86-
throw error(500, "Failed to fetch URL.");
110+
} finally {
111+
clearTimeout(timeoutId);
112+
}
113+
114+
if (!response.ok) {
115+
logger.error({ targetUrl, response }, "Error fetching URL. Response not ok.");
116+
throw error(response.status, `Failed to fetch: ${response.statusText}`);
117+
}
118+
119+
// Check content length if available
120+
const contentLength = response.headers.get("content-length");
121+
if (contentLength && parseInt(contentLength) > MAX_FILE_SIZE) {
122+
throw error(413, "File too large (max 10MB)");
87123
}
124+
125+
// Stream the response back
126+
const originalContentType = response.headers.get("content-type") || "application/octet-stream";
127+
// Send as text/plain for safety; expose the original type via secondary header
128+
const safeContentType = "text/plain; charset=utf-8";
129+
const contentDisposition = response.headers.get("content-disposition");
130+
131+
const headers: HeadersInit = {
132+
"Content-Type": safeContentType,
133+
"X-Forwarded-Content-Type": originalContentType,
134+
"Cache-Control": "public, max-age=3600",
135+
...(contentDisposition ? { "Content-Disposition": contentDisposition } : {}),
136+
...SECURITY_HEADERS,
137+
};
138+
139+
// Get the body as array buffer to check size
140+
const arrayBuffer = await response.arrayBuffer();
141+
142+
if (arrayBuffer.byteLength > MAX_FILE_SIZE) {
143+
throw error(413, "File too large (max 10MB)");
144+
}
145+
146+
return new Response(arrayBuffer, { headers });
88147
}

0 commit comments

Comments
 (0)