Skip to content

Commit 0896bc5

Browse files
dahliadahlia
authored
Merge pull request #913 from dahlia/bugfix/html-document-loader
2 parents 11852d9 + aff4bb5 commit 0896bc5

3 files changed

Lines changed: 189 additions & 14 deletions

File tree

‎CHANGES.md‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,17 @@ To be released.
2020
[#826]: https://github.com/fedify-dev/fedify/issues/826
2121
[#850]: https://github.com/fedify-dev/fedify/pull/850
2222

23+
### @fedify/vocab-runtime
24+
25+
- Changed `getDocumentLoader()` to reject HTML and XHTML responses that do
26+
not advertise an ActivityPub alternate document with a `FetchError`
27+
instead of attempting to parse the HTML as JSON. This makes remote HTML
28+
error pages surface as document loading failures with the response URL and
29+
content type, rather than generic JSON parser crashes. [[#912], [#913]]
30+
31+
[#912]: https://github.com/fedify-dev/fedify/issues/912
32+
[#913]: https://github.com/fedify-dev/fedify/pull/913
33+
2334

2435
Version 2.3.1
2536
-------------

‎packages/vocab-runtime/src/docloader.test.ts‎

Lines changed: 102 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import fetchMock from "fetch-mock";
22
import { deepStrictEqual, ok, rejects } from "node:assert";
33
import { test } from "node:test";
44
import preloadedContexts from "./contexts.ts";
5-
import { getDocumentLoader } from "./docloader.ts";
5+
import { getDocumentLoader, getRemoteDocument } from "./docloader.ts";
66
import { FetchError } from "./request.ts";
77
import { UrlError } from "./url.ts";
88

@@ -90,7 +90,7 @@ test("getDocumentLoader()", async (t) => {
9090
type: "Object",
9191
},
9292
headers: {
93-
"Content-Type": "text/html; charset=utf-8",
93+
"Content-Type": "application/activity+json",
9494
Link: '<https://example.com/object>; rel="alternate"; ' +
9595
'type="application/ld+json; profile="https://www.w3.org/ns/activitystreams""',
9696
},
@@ -247,6 +247,44 @@ test("getDocumentLoader()", async (t) => {
247247
});
248248
});
249249

250+
fetchMock.get("https://example.com/html-no-alternate", {
251+
body: `<!DOCTYPE html>
252+
<html>
253+
<head>
254+
<title>Not an ActivityPub document</title>
255+
</head>
256+
<body>Not found</body>
257+
</html>`,
258+
headers: { "Content-Type": "text/html; charset=utf-8" },
259+
});
260+
261+
await t.test("HTML without ActivityPub alternate link", async () => {
262+
await rejects(
263+
() => fetchDocumentLoader("https://example.com/html-no-alternate"),
264+
(error) => {
265+
ok(error instanceof FetchError);
266+
ok(
267+
error.message.includes(
268+
"HTML document has no ActivityPub alternate link",
269+
),
270+
);
271+
ok(
272+
error.message.includes("Content-Type: text/html; charset=utf-8"),
273+
);
274+
deepStrictEqual(
275+
error.url,
276+
new URL("https://example.com/html-no-alternate"),
277+
);
278+
ok(error.response != null);
279+
deepStrictEqual(
280+
error.response.headers.get("Content-Type"),
281+
"text/html; charset=utf-8",
282+
);
283+
return true;
284+
},
285+
);
286+
});
287+
250288
fetchMock.get("https://example.com/wrong-content-type", {
251289
body: {
252290
"@context": "https://www.w3.org/ns/activitystreams",
@@ -257,7 +295,7 @@ test("getDocumentLoader()", async (t) => {
257295
headers: { "Content-Type": "text/html; charset=utf-8" },
258296
});
259297

260-
await t.test("Wrong Content-Type", async () => {
298+
await t.test("wrong Content-Type with JSON body", async () => {
261299
deepStrictEqual(
262300
await fetchDocumentLoader("https://example.com/wrong-content-type"),
263301
{
@@ -273,6 +311,64 @@ test("getDocumentLoader()", async (t) => {
273311
);
274312
});
275313

314+
fetchMock.get("https://example.com/large-html", {
315+
body: "<!DOCTYPE html>",
316+
headers: {
317+
"Content-Length": String(1024 * 1024 + 1),
318+
"Content-Type": "text/html; charset=utf-8",
319+
},
320+
});
321+
322+
await t.test("HTML Content-Length over limit", async () => {
323+
await rejects(
324+
() => fetchDocumentLoader("https://example.com/large-html"),
325+
(error) => {
326+
ok(error instanceof FetchError);
327+
ok(
328+
error.message.includes(
329+
"HTML document is too large to scan for an ActivityPub alternate link",
330+
),
331+
);
332+
ok(error.response != null);
333+
deepStrictEqual(error.response.status, 200);
334+
deepStrictEqual(
335+
error.response.headers.get("Content-Type"),
336+
"text/html; charset=utf-8",
337+
);
338+
return true;
339+
},
340+
);
341+
});
342+
343+
await t.test("HTML Content-Length over limit cancels body", async () => {
344+
let canceled = false;
345+
const response = new Response("<!DOCTYPE html>", {
346+
headers: {
347+
"Content-Length": String(1024 * 1024 + 1),
348+
"Content-Type": "text/html; charset=utf-8",
349+
},
350+
});
351+
Object.defineProperty(response, "body", {
352+
value: {
353+
cancel: () => {
354+
canceled = true;
355+
},
356+
},
357+
});
358+
await rejects(
359+
() =>
360+
getRemoteDocument(
361+
"https://example.com/large-html-cancel",
362+
response,
363+
() => {
364+
throw new Error("unexpected alternate fetch");
365+
},
366+
),
367+
FetchError,
368+
);
369+
deepStrictEqual(canceled, true);
370+
});
371+
276372
fetchMock.get("https://example.com/404", { status: 404 });
277373

278374
await t.test("not ok", async () => {
@@ -459,11 +555,11 @@ test("getDocumentLoader()", async (t) => {
459555

460556
await t.test("ReDoS resistance (CVE-2025-68475)", async () => {
461557
const start = performance.now();
462-
// The malicious HTML will fail JSON parsing, but the important thing is
463-
// that it should complete quickly (not hang due to ReDoS)
558+
// The malicious HTML will fail alternate discovery, but the important
559+
// thing is that it should complete quickly (not hang due to ReDoS).
464560
await rejects(
465561
() => fetchDocumentLoader("https://example.com/redos"),
466-
SyntaxError,
562+
FetchError,
467563
);
468564
const elapsed = performance.now() - start;
469565

‎packages/vocab-runtime/src/docloader.ts‎

Lines changed: 76 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { UrlError, validatePublicUrl } from "./url.ts";
1313

1414
const logger = getLogger(["fedify", "runtime", "docloader"]);
1515
const DEFAULT_MAX_REDIRECTION = 20;
16+
const MAX_HTML_SIZE = 1024 * 1024; // 1MB
1617

1718
/**
1819
* A remote JSON-LD document and its context fetched by
@@ -112,6 +113,59 @@ export type AuthenticatedDocumentLoaderFactory = (
112113
options?: DocumentLoaderFactoryOptions,
113114
) => DocumentLoader;
114115

116+
function createResponseMetadata(response: Response): Response {
117+
return new Response(null, {
118+
headers: response.headers,
119+
status: response.status,
120+
statusText: response.statusText,
121+
});
122+
}
123+
124+
async function cancelResponseBody(response: Response): Promise<void> {
125+
if (response.body != null) {
126+
await response.body.cancel();
127+
}
128+
}
129+
130+
async function readBoundedText(
131+
response: Response,
132+
maxBytes: number,
133+
): Promise<{ text: string; size: number; tooLarge: boolean }> {
134+
const contentLength = response.headers.get("Content-Length");
135+
if (contentLength != null) {
136+
const size = Number(contentLength);
137+
if (size > maxBytes) {
138+
await cancelResponseBody(response);
139+
return { text: "", size, tooLarge: true };
140+
}
141+
}
142+
143+
if (response.body == null) return { text: "", size: 0, tooLarge: false };
144+
145+
const reader = response.body.getReader();
146+
const decoder = new TextDecoder();
147+
let text = "";
148+
let size = 0;
149+
try {
150+
while (true) {
151+
const result = await reader.read();
152+
if (result.done) break;
153+
const chunkSize = result.value.byteLength;
154+
if (size + chunkSize > maxBytes) {
155+
size += chunkSize;
156+
await reader.cancel();
157+
return { text: "", size, tooLarge: true };
158+
}
159+
size += chunkSize;
160+
text += decoder.decode(result.value, { stream: true });
161+
}
162+
text += decoder.decode();
163+
return { text, size, tooLarge: false };
164+
} finally {
165+
reader.releaseLock();
166+
}
167+
}
168+
115169
/**
116170
* Gets a {@link RemoteDocument} from the given response.
117171
* @param url The URL of the document to load.
@@ -200,15 +254,19 @@ export async function getRemoteDocument(
200254
contentType === "application/xhtml+xml" ||
201255
contentType?.startsWith("application/xhtml+xml;"))
202256
) {
203-
// Security: Limit HTML response size to mitigate ReDoS attacks
204-
const MAX_HTML_SIZE = 1024 * 1024; // 1MB
205-
const html = await response.text();
206-
if (html.length > MAX_HTML_SIZE) {
257+
const errorResponse = createResponseMetadata(response);
258+
const html = await readBoundedText(response, MAX_HTML_SIZE);
259+
if (html.tooLarge) {
207260
logger.warn(
208261
"HTML response too large, skipping alternate link discovery: {url}",
209-
{ url: documentUrl, size: html.length },
262+
{ url: documentUrl, size: html.size },
263+
);
264+
throw new FetchError(
265+
documentUrl,
266+
`HTML document is too large to scan for an ActivityPub alternate link ` +
267+
`(Content-Type: ${contentType})`,
268+
errorResponse,
210269
);
211-
document = JSON.parse(html);
212270
} else {
213271
// Safe regex patterns without nested quantifiers to prevent ReDoS
214272
// (CVE-2025-68475)
@@ -219,7 +277,7 @@ export async function getRemoteDocument(
219277
/([a-z][a-z:_-]*)=(?:"([^"]*)"|'([^']*)'|([^\s>]+))/gi;
220278

221279
let tagMatch: RegExpExecArray | null;
222-
while ((tagMatch = tagPattern.exec(html)) !== null) {
280+
while ((tagMatch = tagPattern.exec(html.text)) !== null) {
223281
const tagContent = tagMatch[2];
224282
let attrMatch: RegExpExecArray | null;
225283
const attribs: Record<string, string> = {};
@@ -247,7 +305,17 @@ export async function getRemoteDocument(
247305
return await fetch(new URL(attribs.href, docUrl).href);
248306
}
249307
}
250-
document = JSON.parse(html);
308+
try {
309+
document = JSON.parse(html.text);
310+
} catch (error) {
311+
if (!(error instanceof SyntaxError)) throw error;
312+
throw new FetchError(
313+
documentUrl,
314+
`HTML document has no ActivityPub alternate link ` +
315+
`(Content-Type: ${contentType})`,
316+
errorResponse,
317+
);
318+
}
251319
}
252320
} else {
253321
document = await response.json();

0 commit comments

Comments
 (0)