Skip to content

Commit edef281

Browse files
authored
fix(security): validate feed/URI URLs and cap download size (#223)
Feed enclosure URLs (<enclosure url>) and obsidian://podnotes deep-link URLs are fully attacker-controlled, yet flowed straight into requestUrl with no scheme or host validation - a blind SSRF primitive (plus a file:/data: local-read/exfil path via the transcription fetch). The streaming download also had no total-size cap, so a malicious media server could fill the disk. - Add src/utility/assertFetchableUrl.ts: allow only http(s); reject loopback/link-local/private/cloud-metadata hosts (IPv4 + IPv6, including obfuscated, IPv4-mapped, and absolute-FQDN forms). - Route every feed/URI-derived fetch through it: downloadFile and the getFileExtension HEAD probe (downloadEpisode.ts), probeAndFetchFirstChunk and writeStreamedFile (download/streaming.ts), and the feed-fetch branch of the obsidian://podnotes handler (URIHandler.ts). - Cap the streamed download at MAX_DOWNLOAD_SIZE (2 GiB): reject an oversized advertised total or 200-fallback body up front, and abort the 206 loop once the running total exceeds the cap (stops the unknown-total infinite-stream disk-fill). deepsec: ssrf-2306e54f4d, ssrf-47eda4095e, ssrf-594c984897, other-resource-exhaustion-54d62080a1
1 parent 989e8bb commit edef281

8 files changed

Lines changed: 474 additions & 0 deletions

src/URIHandler.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,33 @@ describe("podNotesURIHandler", () => {
171171
expect(get(viewState)).toBe(ViewState.PodcastGrid);
172172
});
173173

174+
test.each([
175+
"http://169.254.169.254/latest/meta-data/",
176+
"http://127.0.0.1:8080/feed.xml",
177+
"http://192.168.0.1/feed.xml",
178+
])(
179+
"refuses a deep link whose url points at an internal host (%s) without fetching",
180+
async (url) => {
181+
const revealPlayer = vi.fn();
182+
183+
await podNotesURIHandler(
184+
{
185+
action: "podnotes",
186+
url,
187+
episodeName: "Some Episode",
188+
},
189+
api as never,
190+
revealPlayer,
191+
);
192+
193+
// The attacker-controlled url is never handed to the feed parser.
194+
expect(mockGetEpisodes).not.toHaveBeenCalled();
195+
expect(get(currentEpisode)).toBeUndefined();
196+
expect(get(viewState)).toBe(ViewState.PodcastGrid);
197+
expect(revealPlayer).not.toHaveBeenCalled();
198+
},
199+
);
200+
174201
test("keeps the requested segment end for the player to apply after loading metadata", async () => {
175202
await podNotesURIHandler(
176203
{

src/URIHandler.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
viewState,
1717
} from "./store";
1818
import type { Episode } from "./types/Episode";
19+
import { isFetchableUrl } from "./utility/assertFetchableUrl";
1920
import { getEpisodeKey } from "./utility/episodeKey";
2021
import { ViewState } from "./types/ViewState";
2122

@@ -178,6 +179,17 @@ export default async function podNotesURIHandler(
178179
.map((name) => localFiles.getLocalEpisode(name))
179180
.find((ep) => ep !== undefined);
180181
} else {
182+
// The url here came straight from an untrusted obsidian://podnotes deep link
183+
// (an attacker can put one behind <a href> on a web page), so a single click
184+
// must not be able to fetch an arbitrary internal host. Refuse anything that
185+
// isn't a public http(s) URL before handing it to FeedParser (blind SSRF).
186+
if (!isFetchableUrl(url)) {
187+
new Notice(
188+
"Refusing to load a feed from a private, local, or non-http(s) URL",
189+
);
190+
return;
191+
}
192+
181193
try {
182194
// Fetch with the raw url (current-format links are correct as-is); only the title gets
183195
// the legacy-candidate treatment. A '+' in a legacy feed URL is pre-existing and out of

src/download/streaming.test.ts

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,89 @@ describe("writeStreamedFile", () => {
188188
});
189189
});
190190

191+
describe("download size cap (resource exhaustion)", () => {
192+
it("rejects a 206 whose advertised total exceeds the cap, up front", async () => {
193+
requestUrlMock.mockResolvedValue(
194+
res(206, [1, 2, 3, 4], {
195+
"content-type": "audio/mpeg",
196+
"content-range": "bytes 0-3/1099511627776", // 1 TiB
197+
}),
198+
);
199+
200+
await expect(
201+
probeAndFetchFirstChunk("https://x/ep.mp3", 4, 100),
202+
).rejects.toThrow(/maximum allowed size/);
203+
});
204+
205+
it("rejects a 200 fallback whose whole body exceeds the cap", async () => {
206+
// Server ignores Range and returns the entire (oversized) body in one 200.
207+
requestUrlMock.mockResolvedValue(
208+
res(200, [1, 2, 3, 4, 5, 6, 7, 8, 9, 10], { "content-type": "audio/mpeg" }),
209+
);
210+
211+
await expect(
212+
probeAndFetchFirstChunk("https://x/ep.mp3", 4, 5),
213+
).rejects.toThrow(/maximum allowed size/);
214+
});
215+
216+
it("aborts the 206 loop once the running total exceeds the cap (unknown-total infinite stream)", async () => {
217+
const a = setupAdapter();
218+
// A malicious server with an unknown total ("/*") that returns full-size
219+
// chunks forever — only the running-total cap can stop it.
220+
requestUrlMock.mockResolvedValue(res(206, [9, 9]));
221+
222+
await expect(
223+
writeStreamedFile(
224+
"https://x/ep.mp3",
225+
"out.mp3",
226+
probe({ totalSize: null, firstChunk: new Uint8Array([1, 2]).buffer }),
227+
undefined,
228+
2, // chunkSize
229+
5, // maxSize
230+
),
231+
).rejects.toThrow(/maximum allowed size/);
232+
233+
// It stopped instead of writing without bound.
234+
expect((a.writes.get("out.mp3")?.length ?? 0)).toBeLessThanOrEqual(5 + 2);
235+
});
236+
237+
it("rejects when even the first chunk already exceeds the cap", async () => {
238+
const a = setupAdapter();
239+
240+
await expect(
241+
writeStreamedFile(
242+
"https://x/ep.mp3",
243+
"out.mp3",
244+
probe({ firstChunk: new Uint8Array([1, 2, 3, 4, 5, 6]).buffer, supportsRange: false }),
245+
undefined,
246+
2,
247+
5,
248+
),
249+
).rejects.toThrow(/maximum allowed size/);
250+
expect(a.writeBinary).not.toHaveBeenCalled();
251+
});
252+
});
253+
254+
describe("SSRF guard", () => {
255+
it.each([
256+
"http://169.254.169.254/latest/meta-data/",
257+
"http://127.0.0.1:8080/ep.mp3",
258+
"file:///Users/victim/.ssh/id_rsa",
259+
])("probeAndFetchFirstChunk refuses %s without issuing a request", async (url) => {
260+
await expect(probeAndFetchFirstChunk(url, 4)).rejects.toThrow(/Refusing/);
261+
expect(requestUrlMock).not.toHaveBeenCalled();
262+
});
263+
264+
it("writeStreamedFile refuses a blocked URL before touching the adapter", async () => {
265+
const a = setupAdapter();
266+
await expect(
267+
writeStreamedFile("http://192.168.0.1/ep.mp3", "out.mp3", probe(), undefined, 2),
268+
).rejects.toThrow(/Refusing/);
269+
expect(a.writeBinary).not.toHaveBeenCalled();
270+
expect(requestUrlMock).not.toHaveBeenCalled();
271+
});
272+
});
273+
191274
describe("partialPathFor / isPartialPath", () => {
192275
it("builds a dot-prefixed sibling temp in the same folder", () => {
193276
const tmp = partialPathFor("Podcasts/Show/Ep 1.mp3");

src/download/streaming.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { type DataAdapter, requestUrl } from "obsidian";
22
import { get } from "svelte/store";
33
import { plugin } from "../store";
4+
import { assertFetchableUrl } from "../utility/assertFetchableUrl";
45
import { encodeUrlForRequest } from "../utility/encodeUrlForRequest";
56
import { enforceMaxPathLength } from "../utility/enforceMaxPathLength";
67

@@ -25,6 +26,23 @@ import { enforceMaxPathLength } from "../utility/enforceMaxPathLength";
2526

2627
export const DOWNLOAD_CHUNK_SIZE = 4 * 1024 * 1024; // 4 MiB per range request
2728

29+
// Total-bytes ceiling for a single download. The per-chunk bound above keeps
30+
// peak memory flat, but without a TOTAL cap a malicious media server (the host of
31+
// a feed's enclosure URL is attacker-controlled) can fill the disk: it can answer
32+
// 200 with an enormous body, advertise an arbitrarily large Content-Range total,
33+
// or - with an unknown total ("bytes 0-N/*") - return full-size 206 chunks forever
34+
// so the append loop never terminates. 2 GiB clears any real podcast episode
35+
// (long-form audio is ~hundreds of MB; even large video episodes fit) while
36+
// turning the unbounded write into a bounded, recoverable failure.
37+
export const MAX_DOWNLOAD_SIZE = 2 * 1024 * 1024 * 1024; // 2 GiB
38+
39+
function tooLargeError(maxSize: number): Error {
40+
const maxMb = Math.round(maxSize / (1024 * 1024));
41+
return new Error(
42+
`Download exceeds the maximum allowed size (${maxMb} MB). Aborting.`,
43+
);
44+
}
45+
2846
// Obsidian's DataAdapter — writeBinary/rename/remove/list, all used below — is
2947
// fully typed and public. Only `appendBinary` exists at runtime on the desktop and
3048
// mobile Capacitor adapters without appearing in the public typings, so we extend
@@ -69,7 +87,9 @@ function readHeader(
6987
export async function probeAndFetchFirstChunk(
7088
url: string,
7189
chunkSize: number = DOWNLOAD_CHUNK_SIZE,
90+
maxSize: number = MAX_DOWNLOAD_SIZE,
7291
): Promise<RangeProbe> {
92+
assertFetchableUrl(url);
7393
const encodedUrl = encodeUrlForRequest(url);
7494
const response = await requestUrl({
7595
url: encodedUrl,
@@ -104,6 +124,17 @@ export async function probeAndFetchFirstChunk(
104124
}
105125
}
106126

127+
// Reject an oversized download up front: a known total over the cap, or a 200
128+
// fallback whose whole body (requestUrl has already buffered it) is over the
129+
// cap. The unknown-total 206 case can't be caught here and is bounded by the
130+
// running-total check in writeStreamedFile instead.
131+
if (totalSize !== null && totalSize > maxSize) {
132+
throw tooLargeError(maxSize);
133+
}
134+
if (!supportsRange && response.arrayBuffer.byteLength > maxSize) {
135+
throw tooLargeError(maxSize);
136+
}
137+
107138
return {
108139
firstChunk: response.arrayBuffer,
109140
contentType,
@@ -123,9 +154,15 @@ export async function writeStreamedFile(
123154
probe: RangeProbe,
124155
onProgress?: (written: number, total: number | null) => void,
125156
chunkSize: number = DOWNLOAD_CHUNK_SIZE,
157+
maxSize: number = MAX_DOWNLOAD_SIZE,
126158
): Promise<number> {
159+
assertFetchableUrl(url);
127160
const adapter = appendableAdapter();
128161

162+
if (probe.firstChunk.byteLength > maxSize) {
163+
throw tooLargeError(maxSize);
164+
}
165+
129166
await adapter.writeBinary(destPath, probe.firstChunk);
130167
let written = probe.firstChunk.byteLength;
131168
onProgress?.(written, probe.totalSize);
@@ -166,6 +203,14 @@ export async function writeStreamedFile(
166203
written += chunk.byteLength;
167204
onProgress?.(written, probe.totalSize);
168205

206+
// Hard ceiling on the total written. This is the only stop for a server
207+
// that advertises an unknown total ("bytes 0-N/*") and returns full-size
208+
// 206 chunks forever — without it the loop would append to disk until the
209+
// disk fills. The caller's finally drops the (now over-cap) temp file.
210+
if (written > maxSize) {
211+
throw tooLargeError(maxSize);
212+
}
213+
169214
// Unknown total: a short chunk means we hit EOF.
170215
if (probe.totalSize === null && chunk.byteLength < chunkSize) break;
171216
}

src/downloadEpisode.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1225,6 +1225,23 @@ describe("getEpisodeAudioBuffer (issue #107)", () => {
12251225
expect(result.basename).toBe("recording");
12261226
expect(requestUrlMock).not.toHaveBeenCalled();
12271227
});
1228+
1229+
it.each([
1230+
"file:///Users/victim/.ssh/id_rsa",
1231+
"http://169.254.169.254/latest/meta-data/",
1232+
"http://127.0.0.1:9200/_search",
1233+
])(
1234+
"refuses to fetch a feed-controlled stream URL pointing at %s (SSRF/exfil guard)",
1235+
async (streamUrl) => {
1236+
const ssrf = episode({
1237+
title: "Malicious Enclosure",
1238+
streamUrl,
1239+
});
1240+
1241+
await expect(getEpisodeAudioBuffer(ssrf)).rejects.toThrow(/Refusing/);
1242+
expect(requestUrlMock).not.toHaveBeenCalled();
1243+
},
1244+
);
12281245
});
12291246

12301247
describe("downloadEpisodeWithNotice (streaming range path)", () => {

src/downloadEpisode.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
} from "./TemplateEngine";
88
import type { Episode, EpisodeMediaType } from "./types/Episode";
99
import type { LocalEpisode } from "./types/LocalEpisode";
10+
import { assertFetchableUrl } from "./utility/assertFetchableUrl";
1011
import { encodeUrlForRequest } from "./utility/encodeUrlForRequest";
1112
import { enforceMaxPathLength } from "./utility/enforceMaxPathLength";
1213
import { ensureFolderExists } from "./utility/ensureFolderExists";
@@ -53,6 +54,7 @@ interface DownloadedFile {
5354
// and transcription's getEpisodeAudioBuffer still need the entire buffer at once.
5455
// The Download command streams instead — see downloadEpisodeToDisk.
5556
async function downloadFile(url: string): Promise<DownloadedFile> {
57+
assertFetchableUrl(url);
5658
const encodedUrl = encodeUrlForRequest(url);
5759
try {
5860
const response = await requestUrl({ url: encodedUrl, method: "GET" });
@@ -708,6 +710,7 @@ export async function downloadEpisode(
708710
}
709711

710712
async function getFileExtension(url: string): Promise<string> {
713+
assertFetchableUrl(url);
711714
const encodedUrl = encodeUrlForRequest(url);
712715
const urlExtension = getUrlExtension(encodedUrl);
713716
if (urlExtension) return urlExtension;
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
import { describe, expect, it } from "vitest";
2+
import {
3+
assertFetchableUrl,
4+
isFetchableUrl,
5+
UnsafeFetchUrlError,
6+
} from "./assertFetchableUrl";
7+
8+
describe("assertFetchableUrl", () => {
9+
it("accepts ordinary public http(s) feed/enclosure URLs", () => {
10+
expect(() =>
11+
assertFetchableUrl("https://pod.example.com/audio.mp3?token=abc"),
12+
).not.toThrow();
13+
expect(() =>
14+
assertFetchableUrl("http://cdn.example.org/ep/1.mp3"),
15+
).not.toThrow();
16+
// A public IP is fine.
17+
expect(() => assertFetchableUrl("https://8.8.8.8/feed.xml")).not.toThrow();
18+
});
19+
20+
it("returns the parsed URL", () => {
21+
const url = assertFetchableUrl("https://example.com/a.mp3");
22+
expect(url.hostname).toBe("example.com");
23+
});
24+
25+
it.each([
26+
"file:///Users/victim/.ssh/id_rsa",
27+
"data:text/plain,hello",
28+
"blob:https://example.com/uuid",
29+
"ftp://example.com/file",
30+
"javascript:alert(1)",
31+
])("rejects non-http(s) scheme %s", (url) => {
32+
expect(() => assertFetchableUrl(url)).toThrow(UnsafeFetchUrlError);
33+
});
34+
35+
it.each([
36+
"",
37+
" ",
38+
"not a url",
39+
"//example.com/no-scheme",
40+
])("rejects empty/malformed url %s", (url) => {
41+
expect(() => assertFetchableUrl(url)).toThrow(UnsafeFetchUrlError);
42+
});
43+
44+
it.each([
45+
"http://localhost/feed.xml",
46+
"http://LOCALHOST:8080/x",
47+
"http://api.localhost/x",
48+
"http://localhost./x", // absolute-FQDN form still resolves to loopback
49+
"http://LOCALHOST./x",
50+
"http://sub.localhost./x",
51+
"http://127.0.0.1/x",
52+
"http://127.1.2.3/x",
53+
"http://10.0.0.5/x",
54+
"http://172.16.0.1/x",
55+
"http://172.31.255.255/x",
56+
"http://192.168.1.5/x",
57+
"http://169.254.169.254/latest/meta-data/", // cloud metadata
58+
"http://0.0.0.0/x",
59+
])("blocks loopback/private/link-local host %s", (url) => {
60+
expect(() => assertFetchableUrl(url)).toThrow(UnsafeFetchUrlError);
61+
});
62+
63+
it.each([
64+
"http://2130706433/x", // 127.0.0.1 as integer
65+
"http://0x7f000001/x", // 127.0.0.1 as hex
66+
"http://0177.0.0.1/x", // 127.0.0.1 with octal leading octet
67+
"http://0/x", // 0.0.0.0
68+
])("blocks obfuscated IPv4 loopback %s", (url) => {
69+
expect(() => assertFetchableUrl(url)).toThrow(UnsafeFetchUrlError);
70+
});
71+
72+
it.each([
73+
"http://[::1]/x", // loopback
74+
"http://[::]/x", // unspecified
75+
"http://[fe80::1]/x", // link-local
76+
"http://[fc00::1]/x", // unique-local
77+
"http://[fd12:3456:789a::1]/x", // unique-local
78+
"http://[::ffff:127.0.0.1]/x", // IPv4-mapped loopback
79+
"http://[::ffff:169.254.169.254]/x", // IPv4-mapped metadata
80+
"http://[::ffff:7f00:1]/x", // IPv4-mapped loopback in hex form
81+
])("blocks loopback/private IPv6 host %s", (url) => {
82+
expect(() => assertFetchableUrl(url)).toThrow(UnsafeFetchUrlError);
83+
});
84+
85+
it.each([
86+
"https://[2606:4700:4700::1111]/x", // public IPv6 (Cloudflare DNS)
87+
"https://203.0.113.10/x", // public IPv4
88+
"https://pod.example.com./feed.xml", // legit absolute FQDN is not blocked
89+
])("allows public IPv6/IPv4/FQDN host %s", (url) => {
90+
expect(() => assertFetchableUrl(url)).not.toThrow();
91+
});
92+
93+
it("exposes a non-throwing predicate", () => {
94+
expect(isFetchableUrl("https://example.com/a.mp3")).toBe(true);
95+
expect(isFetchableUrl("http://169.254.169.254/")).toBe(false);
96+
expect(isFetchableUrl("file:///etc/passwd")).toBe(false);
97+
});
98+
});

0 commit comments

Comments
 (0)