From 52483123b94b6b41acf4581e097f3e713a34c726 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Wed, 5 Aug 2026 11:26:13 +0000 Subject: [PATCH] Use safeFetch for Downloader API connections to prevent SSRF Co-authored-by: Doezer <11655673+Doezer@users.noreply.github.com> --- .jules/sentinel.md | 7 ++++++- server/__tests__/downloaders_deluge.test.ts | 1 + server/__tests__/downloaders_deluge_coverage.test.ts | 1 + server/__tests__/downloaders_deluge_remaining.test.ts | 4 ++++ server/__tests__/downloaders_helpers_regression.test.ts | 5 +++++ server/__tests__/downloaders_nzbget_remaining.test.ts | 2 +- .../__tests__/downloaders_qbittorrent_remaining.test.ts | 2 +- server/__tests__/downloaders_rtorrent_remaining.test.ts | 1 + .../__tests__/downloaders_transmission_remaining.test.ts | 1 + server/__tests__/routes.test.ts | 4 ++++ server/downloaders/deluge.ts | 4 ++-- server/downloaders/nzbget.ts | 2 +- server/downloaders/qbittorrent.ts | 8 ++++---- server/downloaders/rtorrent.ts | 6 +++--- server/downloaders/transmission.ts | 6 +++--- 15 files changed, 38 insertions(+), 16 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index a220ac68f..7dfd5970d 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -22,8 +22,13 @@ **Learning:** Checking for string prefixes on URLs is insufficient for security because the userinfo component (e.g. `https://discord.com/api/webhooks/@127.0.0.1`) can be used to bypass the check. The `fetch` API and URL parsers will interpret `127.0.0.1` as the hostname and `discord.com` as the credentials. **Prevention:** Never rely on string prefix matching for URL validation. Always use the project's `safeFetch` utility which properly resolves and validates the target IP address to prevent SSRF and DNS rebinding attacks. -## $(date +%Y-%m-%d) - Prevent SSRF by validating parsed URL components instead of prefix matching +## 2025-05-24 - Prevent SSRF by validating parsed URL components instead of prefix matching **Vulnerability:** The application validated Discord webhook URLs (and potentially other external URLs) by matching the string prefix (`url.startsWith("https://discord.com")`). This is vulnerable to SSRF bypasses via the userinfo component (e.g., `https://discord.com@127.0.0.1/api/webhooks/`). **Learning:** Checking string prefixes for URL validation is fundamentally insecure because parts of the prefix might be interpreted as the username/password in a URL with a different domain. Attackers can leverage this to bypass domain allowlists. **Prevention:** Always use the `URL` object (e.g., `new URL()`) to parse URLs and explicitly validate the `hostname` and `pathname` properties instead of checking raw string prefixes. + +## 2025-05-24 - SSRF Prevention in Internal Downstream Requests +**Vulnerability:** Downloader API communication (e.g. qBittorrent, Transmission, NZBGet, Deluge, rTorrent) was using the native `fetch()` to call external URLs (e.g. downloaders running locally or remotely), making the application vulnerable to Server-Side Request Forgery (SSRF) and DNS Rebinding via metadata or local networks. +**Learning:** Even internal API connections must be shielded from interacting with sensitive hostnames or local/cloud metadata networks (169.254.169.254, etc.) via DNS rebinding. Replacing native `fetch` with a secure wrapper ensures strict host-level validation of all connections. +**Prevention:** Always use the local `safeFetch` implementation from `server/ssrf.ts` instead of the global `fetch()` when making external network requests to any user-provided URL or API configuration to prevent SSRF vulnerabilities. diff --git a/server/__tests__/downloaders_deluge.test.ts b/server/__tests__/downloaders_deluge.test.ts index cf2204655..d2d89f6f8 100644 --- a/server/__tests__/downloaders_deluge.test.ts +++ b/server/__tests__/downloaders_deluge.test.ts @@ -19,6 +19,7 @@ vi.mock("../logger.js", () => ({ vi.mock("../ssrf.js", () => ({ isSafeUrl: vi.fn().mockResolvedValue(true), + safeFetch: vi.fn((url: string, options: RequestInit) => fetch(url, options)), })); vi.mock("../downloaders/utils.js", async (importOriginal) => { diff --git a/server/__tests__/downloaders_deluge_coverage.test.ts b/server/__tests__/downloaders_deluge_coverage.test.ts index 7d00b147d..dba4e8ed9 100644 --- a/server/__tests__/downloaders_deluge_coverage.test.ts +++ b/server/__tests__/downloaders_deluge_coverage.test.ts @@ -19,6 +19,7 @@ vi.mock("../logger.js", () => ({ vi.mock("../ssrf.js", () => ({ isSafeUrl: vi.fn().mockResolvedValue(true), + safeFetch: vi.fn((url: string, options: RequestInit) => fetch(url, options)), })); vi.mock("../downloaders/utils.js", async (importOriginal) => { diff --git a/server/__tests__/downloaders_deluge_remaining.test.ts b/server/__tests__/downloaders_deluge_remaining.test.ts index 310f819d5..875976ac0 100644 --- a/server/__tests__/downloaders_deluge_remaining.test.ts +++ b/server/__tests__/downloaders_deluge_remaining.test.ts @@ -18,6 +18,10 @@ vi.mock("../logger.js", () => ({ })); global.fetch = fetchMock as unknown as typeof fetch; +vi.mock("../ssrf.js", () => ({ + isSafeUrl: vi.fn().mockResolvedValue(true), + safeFetch: vi.fn((url: string, options?: RequestInit) => global.fetch(url, options)), +})); const { DelugeClient } = await import("../downloaders/deluge.js"); diff --git a/server/__tests__/downloaders_helpers_regression.test.ts b/server/__tests__/downloaders_helpers_regression.test.ts index 436373e86..620eb9000 100644 --- a/server/__tests__/downloaders_helpers_regression.test.ts +++ b/server/__tests__/downloaders_helpers_regression.test.ts @@ -28,6 +28,11 @@ vi.mock("../logger.js", () => ({ const fetchMock = vi.fn(); global.fetch = fetchMock as unknown as typeof fetch; +vi.mock("../ssrf.js", () => ({ + isSafeUrl: vi.fn().mockResolvedValue(true), + safeFetch: vi.fn((url: string, options?: RequestInit) => global.fetch(url, options)), + resolveSafeAddress: vi.fn().mockResolvedValue({ address: "127.0.0.1", family: 4 }), +})); const createDownloader = (overrides: Partial = {}): Downloader => { const now = new Date("2024-01-01T00:00:00.000Z"); diff --git a/server/__tests__/downloaders_nzbget_remaining.test.ts b/server/__tests__/downloaders_nzbget_remaining.test.ts index 20eed3a05..df5a3da0f 100644 --- a/server/__tests__/downloaders_nzbget_remaining.test.ts +++ b/server/__tests__/downloaders_nzbget_remaining.test.ts @@ -14,7 +14,7 @@ vi.mock("../logger.js", () => ({ vi.mock("../ssrf.js", () => ({ isSafeUrl: vi.fn(), - safeFetch: vi.fn(), + safeFetch: vi.fn((url: string, options: RequestInit) => fetch(url, options)), })); const { isSafeUrl, safeFetch } = await import("../ssrf.js"); diff --git a/server/__tests__/downloaders_qbittorrent_remaining.test.ts b/server/__tests__/downloaders_qbittorrent_remaining.test.ts index 953c9139e..ff5784f72 100644 --- a/server/__tests__/downloaders_qbittorrent_remaining.test.ts +++ b/server/__tests__/downloaders_qbittorrent_remaining.test.ts @@ -21,7 +21,7 @@ vi.mock("../logger.js", () => ({ vi.mock("../ssrf.js", () => ({ isSafeUrl: vi.fn().mockResolvedValue(true), - safeFetch: vi.fn(), + safeFetch: vi.fn((url: string, options?: RequestInit) => global.fetch(url, options)), })); vi.mock("../downloaders/utils.js", async (importOriginal) => { diff --git a/server/__tests__/downloaders_rtorrent_remaining.test.ts b/server/__tests__/downloaders_rtorrent_remaining.test.ts index 390504381..d79827db8 100644 --- a/server/__tests__/downloaders_rtorrent_remaining.test.ts +++ b/server/__tests__/downloaders_rtorrent_remaining.test.ts @@ -22,6 +22,7 @@ vi.mock("../logger.js", () => ({ vi.mock("../ssrf.js", () => ({ isSafeUrl: vi.fn().mockResolvedValue(true), + safeFetch: vi.fn((url: string, options: RequestInit) => fetch(url, options)), })); vi.mock("../downloaders/utils.js", async (importOriginal) => { diff --git a/server/__tests__/downloaders_transmission_remaining.test.ts b/server/__tests__/downloaders_transmission_remaining.test.ts index 51c154e38..0a65d9ebf 100644 --- a/server/__tests__/downloaders_transmission_remaining.test.ts +++ b/server/__tests__/downloaders_transmission_remaining.test.ts @@ -21,6 +21,7 @@ vi.mock("../logger.js", () => ({ vi.mock("../ssrf.js", () => ({ isSafeUrl: vi.fn().mockResolvedValue(true), + safeFetch: vi.fn((url: string, options: RequestInit) => fetch(url, options)), })); vi.mock("../downloaders/utils.js", async (importOriginal) => { diff --git a/server/__tests__/routes.test.ts b/server/__tests__/routes.test.ts index 69bcbe1ac..32d1b6855 100644 --- a/server/__tests__/routes.test.ts +++ b/server/__tests__/routes.test.ts @@ -1,6 +1,10 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import type { Downloader } from "../../shared/schema"; import { DownloaderManager } from "../downloaders.js"; +vi.mock("../ssrf.js", () => ({ + isSafeUrl: vi.fn().mockResolvedValue(true), + safeFetch: vi.fn((url: string, options?: RequestInit) => global.fetch(url, options)), +})); describe("/api/downloads endpoint", () => { let fetchMock: ReturnType; diff --git a/server/downloaders/deluge.ts b/server/downloaders/deluge.ts index 93f3545a0..fd1ed2420 100644 --- a/server/downloaders/deluge.ts +++ b/server/downloaders/deluge.ts @@ -6,7 +6,7 @@ import type { DownloadDetails, } from "../../shared/schema.js"; import { downloadersLogger } from "../logger.js"; -import { isSafeUrl } from "../ssrf.js"; +import { isSafeUrl, safeFetch } from "../ssrf.js"; import type { DownloadRequest, DownloaderClient } from "./types.js"; import { fetchWithMagnetDetection, extractHashFromUrl } from "./utils.js"; import { z } from "zod"; @@ -767,7 +767,7 @@ export class DelugeClient implements DownloaderClient { headers["Cookie"] = this.cookie; } - const response = await fetch(url, { + const response = await safeFetch(url, { method: "POST", headers, body: JSON.stringify(body), diff --git a/server/downloaders/nzbget.ts b/server/downloaders/nzbget.ts index acfacae4f..dab85c829 100644 --- a/server/downloaders/nzbget.ts +++ b/server/downloaders/nzbget.ts @@ -196,7 +196,7 @@ export class NZBGetClient implements DownloaderClient { downloadersLogger.debug({ url, method, params: logParams }, "Making NZBGet XML-RPC request"); - const response = await fetch(url, { + const response = await safeFetch(url, { method: "POST", headers, body: xmlBody, diff --git a/server/downloaders/qbittorrent.ts b/server/downloaders/qbittorrent.ts index 2436288e1..a126e8918 100644 --- a/server/downloaders/qbittorrent.ts +++ b/server/downloaders/qbittorrent.ts @@ -8,7 +8,7 @@ import type { import { downloadersLogger } from "../logger.js"; import { randomUUID } from "node:crypto"; import parseTorrent from "parse-torrent"; -import { isSafeUrl } from "../ssrf.js"; +import { isSafeUrl, safeFetch } from "../ssrf.js"; import type { DownloadRequest, DownloaderClient } from "./types.js"; import { fetchWithMagnetDetection, extractHashFromUrl, fixNzbUrlEncoding } from "./utils.js"; @@ -1189,7 +1189,7 @@ export class QBittorrentClient implements DownloaderClient { formData.append("password", this.downloader.password); try { - const response = await fetch(url, { + const response = await safeFetch(url, { method: "POST", headers: { "Content-Type": "application/x-www-form-urlencoded", @@ -1339,7 +1339,7 @@ export class QBittorrentClient implements DownloaderClient { "Making qBittorrent request" ); - let response = await fetch(url, { + let response = await safeFetch(url, { method, headers, body: requestBody, @@ -1358,7 +1358,7 @@ export class QBittorrentClient implements DownloaderClient { retryHeaders["Cookie"] = this.cookie; } - response = await fetch(url, { + response = await safeFetch(url, { method, headers: retryHeaders, body: requestBody, diff --git a/server/downloaders/rtorrent.ts b/server/downloaders/rtorrent.ts index 9398b1937..41afc2a37 100644 --- a/server/downloaders/rtorrent.ts +++ b/server/downloaders/rtorrent.ts @@ -8,7 +8,7 @@ import type { import { downloadersLogger } from "../logger.js"; import parseTorrent from "parse-torrent"; import crypto from "crypto"; -import { isSafeUrl } from "../ssrf.js"; +import { isSafeUrl, safeFetch } from "../ssrf.js"; import type { DownloadRequest, DownloaderClient, XMLValue } from "./types.js"; import { fetchWithMagnetDetection, extractHashFromUrl } from "./utils.js"; import { XMLParser } from "fast-xml-parser"; @@ -711,7 +711,7 @@ export class RTorrentClient implements DownloaderClient { headers["Authorization"] = `Basic ${auth}`; } - const response = await fetch(url, { + const response = await safeFetch(url, { method: "POST", headers, body: xmlBody, @@ -744,7 +744,7 @@ export class RTorrentClient implements DownloaderClient { downloadersLogger.debug({ url }, "Retrying rTorrent request with Digest Auth"); - const retryResponse = await fetch(url, { + const retryResponse = await safeFetch(url, { method: "POST", headers, body: xmlBody, diff --git a/server/downloaders/transmission.ts b/server/downloaders/transmission.ts index 7e01d2a6b..a802638b6 100644 --- a/server/downloaders/transmission.ts +++ b/server/downloaders/transmission.ts @@ -7,7 +7,7 @@ import type { } from "../../shared/schema.js"; import { downloadersLogger } from "../logger.js"; import parseTorrent from "parse-torrent"; -import { isSafeUrl } from "../ssrf.js"; +import { isSafeUrl, safeFetch } from "../ssrf.js"; import type { DownloadRequest, DownloaderClient } from "./types.js"; import { fetchWithMagnetDetection } from "./utils.js"; @@ -699,7 +699,7 @@ export class TransmissionClient implements DownloaderClient { headers["Authorization"] = `Basic ${auth}`; } - const response = await fetch(url, { + const response = await safeFetch(url, { method: "POST", headers, body: JSON.stringify(body), @@ -716,7 +716,7 @@ export class TransmissionClient implements DownloaderClient { downloadersLogger.debug({ method, url }, "Retrying Transmission request with session ID"); // Retry with session ID - const retryResponse = await fetch(url, { + const retryResponse = await safeFetch(url, { method: "POST", headers, body: JSON.stringify(body),