Skip to content

Commit 03dce37

Browse files
authored
Merge commit from fork
Mitigate redirect-based resource exhaustion in remote key resolution
2 parents 86566cf + fcf7e2b commit 03dce37

7 files changed

Lines changed: 280 additions & 15 deletions

File tree

CHANGES.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,22 @@ Version 1.9.6
88

99
To be released.
1010

11+
### @fedify/fedify
12+
13+
- Limited the number of HTTP redirects followed by the remote document
14+
loaders and signed HTTP fetches to mitigate resource exhaustion during
15+
remote key and document resolution. [[CVE-2026-34148] by Abhinav Jaswal]
16+
17+
- Stopped the remote document loaders and signed HTTP fetches from
18+
revisiting the same URL within a redirect chain, preventing
19+
self-referential redirect loops. [[CVE-2026-34148] by Abhinav Jaswal]
20+
21+
- Persisted negative public key cache entries for failed remote key
22+
fetches, reducing repeated retries against the same unavailable key
23+
across requests. [[CVE-2026-34148] by Abhinav Jaswal]
24+
25+
[CVE-2026-34148]: https://github.com/fedify-dev/fedify/security/advisories/GHSA-gm9m-gwc4-hwgp
26+
1127

1228
Version 1.9.5
1329
-------------

packages/fedify/src/federation/keycache.test.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ test("KvKeyCache.set()", async () => {
3838

3939
await cache.set(new URL("https://example.com/null"), null);
4040
assert(cache.nullKeys.has("https://example.com/null"));
41+
assertEquals(
42+
await kv.get(["pk", "https://example.com/null"]),
43+
{ _fedify: "key-unavailable" },
44+
);
4145
});
4246

4347
test("KvKeyCache.get()", async () => {
@@ -64,4 +68,11 @@ test("KvKeyCache.get()", async () => {
6468

6569
cache.nullKeys.add("https://example.com/null");
6670
assertEquals(await cache.get(new URL("https://example.com/null")), null);
71+
72+
await kv.set(
73+
["pk", "https://example.com/null2"],
74+
{ _fedify: "key-unavailable" },
75+
);
76+
const cache2 = new KvKeyCache(kv, ["pk"]);
77+
assertEquals(await cache2.get(new URL("https://example.com/null2")), null);
6778
});

packages/fedify/src/federation/keycache.ts

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,22 @@ import type { KeyCache } from "../sig/key.ts";
33
import { CryptographicKey, Multikey } from "../vocab/vocab.ts";
44
import type { KvKey, KvStore } from "./kv.ts";
55

6+
const NULL_KEY_CACHE_VALUE = { _fedify: "key-unavailable" };
7+
const NULL_KEY_CACHE_TTL = Temporal.Duration.from({ minutes: 5 });
8+
69
export interface KvKeyCacheOptions {
710
documentLoader?: DocumentLoader;
811
contextLoader?: DocumentLoader;
912
}
1013

14+
function isNullKeyCacheValue(
15+
value: unknown,
16+
): value is typeof NULL_KEY_CACHE_VALUE {
17+
return typeof value === "object" && value != null &&
18+
"_fedify" in value &&
19+
value._fedify === NULL_KEY_CACHE_VALUE._fedify;
20+
}
21+
1122
export class KvKeyCache implements KeyCache {
1223
readonly kv: KvStore;
1324
readonly prefix: KvKey;
@@ -27,6 +38,10 @@ export class KvKeyCache implements KeyCache {
2738
if (this.nullKeys.has(keyId.href)) return null;
2839
const serialized = await this.kv.get([...this.prefix, keyId.href]);
2940
if (serialized == null) return undefined;
41+
if (isNullKeyCacheValue(serialized)) {
42+
this.nullKeys.add(keyId.href);
43+
return null;
44+
}
3045
try {
3146
return await CryptographicKey.fromJsonLd(serialized, this.options);
3247
} catch {
@@ -45,7 +60,11 @@ export class KvKeyCache implements KeyCache {
4560
): Promise<void> {
4661
if (key == null) {
4762
this.nullKeys.add(keyId.href);
48-
await this.kv.delete([...this.prefix, keyId.href]);
63+
await this.kv.set(
64+
[...this.prefix, keyId.href],
65+
NULL_KEY_CACHE_VALUE,
66+
{ ttl: NULL_KEY_CACHE_TTL },
67+
);
4968
return;
5069
}
5170
this.nullKeys.delete(keyId.href);

packages/fedify/src/runtime/docloader.test.ts

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,73 @@ test("getDocumentLoader()", async (t) => {
365365
);
366366
});
367367

368+
let redirectAttempts = 0;
369+
fetchMock.get("begin:https://example.com/too-many-redirects/", (cl) => {
370+
redirectAttempts++;
371+
const index = Number(cl.url.split("/").at(-1));
372+
return {
373+
status: 302,
374+
headers: {
375+
Location: `https://example.com/too-many-redirects/${index + 1}`,
376+
},
377+
};
378+
});
379+
380+
await t.step("too many redirects", async () => {
381+
redirectAttempts = 0;
382+
await assertRejects(
383+
() => fetchDocumentLoader("https://example.com/too-many-redirects/0"),
384+
FetchError,
385+
"Too many redirections",
386+
);
387+
assertEquals(redirectAttempts, 21);
388+
});
389+
390+
let loopAttempts = 0;
391+
fetchMock.get("https://example.com/redirect-loop-a", () => {
392+
loopAttempts++;
393+
return {
394+
status: 302,
395+
headers: { Location: "https://example.com/redirect-loop-b" },
396+
};
397+
});
398+
fetchMock.get("https://example.com/redirect-loop-b", () => {
399+
loopAttempts++;
400+
return {
401+
status: 302,
402+
headers: { Location: "https://example.com/redirect-loop-a" },
403+
};
404+
});
405+
406+
await t.step("redirect loop", async () => {
407+
loopAttempts = 0;
408+
await assertRejects(
409+
() => fetchDocumentLoader("https://example.com/redirect-loop-a"),
410+
FetchError,
411+
"Redirect loop detected",
412+
);
413+
assertEquals(loopAttempts, 2);
414+
});
415+
416+
let relativeLoopAttempts = 0;
417+
fetchMock.get("https://example.com/redirect-loop-relative", () => {
418+
relativeLoopAttempts++;
419+
return {
420+
status: 302,
421+
headers: { Location: "/redirect-loop-relative" },
422+
};
423+
});
424+
425+
await t.step("redirect loop with relative location", async () => {
426+
relativeLoopAttempts = 0;
427+
await assertRejects(
428+
() => fetchDocumentLoader("https://example.com/redirect-loop-relative"),
429+
FetchError,
430+
"Redirect loop detected",
431+
);
432+
assertEquals(relativeLoopAttempts, 1);
433+
});
434+
368435
// Regression test for ReDoS vulnerability (CVE-2025-68475)
369436
// Malicious HTML payload: <a a="b" a="b" ... (unclosed tag)
370437
// With the vulnerable regex, this causes catastrophic backtracking

packages/fedify/src/runtime/docloader.ts

Lines changed: 42 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { HttpHeaderLink } from "./link.ts";
77
import { UrlError, validatePublicUrl } from "./url.ts";
88

99
const logger = getLogger(["fedify", "runtime", "docloader"]);
10+
const DEFAULT_MAX_REDIRECTION = 20;
1011

1112
/**
1213
* A remote JSON-LD document and its context fetched by
@@ -352,27 +353,34 @@ export function getDocumentLoader(
352353
async function load(
353354
url: string,
354355
options?: DocumentLoaderOptions,
356+
redirected = 0,
357+
visited = new Set<string>(),
355358
): Promise<RemoteDocument> {
356359
options?.signal?.throwIfAborted();
357-
if (!skipPreloadedContexts && url in preloadedContexts) {
358-
logger.debug("Using preloaded context: {url}.", { url });
360+
const currentUrl = new URL(url).href;
361+
if (!skipPreloadedContexts && currentUrl in preloadedContexts) {
362+
logger.debug("Using preloaded context: {url}.", { url: currentUrl });
359363
return {
360364
contextUrl: null,
361-
document: preloadedContexts[url],
362-
documentUrl: url,
365+
document: preloadedContexts[currentUrl],
366+
documentUrl: currentUrl,
363367
};
364368
}
365369
if (!allowPrivateAddress) {
366370
try {
367-
await validatePublicUrl(url);
371+
await validatePublicUrl(currentUrl);
368372
} catch (error) {
369373
if (error instanceof UrlError) {
370-
logger.error("Disallowed private URL: {url}", { url, error });
374+
logger.error("Disallowed private URL: {url}", {
375+
url: currentUrl,
376+
error,
377+
});
371378
}
372379
throw error;
373380
}
374381
}
375-
const request = createRequest(url, { userAgent });
382+
visited.add(currentUrl);
383+
const request = createRequest(currentUrl, { userAgent });
376384
logRequest(request);
377385
const response = await fetch(request, {
378386
// Since Bun has a bug that ignores the `Request.redirect` option,
@@ -386,9 +394,34 @@ export function getDocumentLoader(
386394
response.status >= 300 && response.status < 400 &&
387395
response.headers.has("Location")
388396
) {
389-
return load(response.headers.get("Location")!, options);
397+
if (redirected >= DEFAULT_MAX_REDIRECTION) {
398+
logger.error(
399+
"Too many redirections ({redirections}) while fetching document.",
400+
{ redirections: redirected + 1, url: currentUrl },
401+
);
402+
throw new FetchError(
403+
currentUrl,
404+
`Too many redirections (${redirected + 1})`,
405+
);
406+
}
407+
const redirectUrl = new URL(
408+
response.headers.get("Location")!,
409+
response.url === "" ? currentUrl : response.url,
410+
).href;
411+
if (visited.has(redirectUrl)) {
412+
logger.error(
413+
"Detected a redirect loop while fetching document: {url} -> " +
414+
"{redirectUrl}",
415+
{ url: currentUrl, redirectUrl },
416+
);
417+
throw new FetchError(
418+
currentUrl,
419+
`Redirect loop detected: ${redirectUrl}`,
420+
);
421+
}
422+
return load(redirectUrl, options, redirected + 1, visited);
390423
}
391-
return getRemoteDocument(url, response, load);
424+
return getRemoteDocument(currentUrl, response, load);
392425
}
393426
return load;
394427
}

packages/fedify/src/sig/http.test.ts

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1650,6 +1650,82 @@ test("doubleKnock() complex redirect chain test", async () => {
16501650
fetchMock.hardReset();
16511651
});
16521652

1653+
test("doubleKnock() throws on too many redirects", async () => {
1654+
fetchMock.spyGlobal();
1655+
1656+
let requestCount = 0;
1657+
fetchMock.post("begin:https://example.com/too-many-redirects/", (cl) => {
1658+
requestCount++;
1659+
const index = Number(cl.url.split("/").at(-1));
1660+
return Response.redirect(
1661+
`https://example.com/too-many-redirects/${index + 1}`,
1662+
302,
1663+
);
1664+
});
1665+
1666+
const request = new Request("https://example.com/too-many-redirects/0", {
1667+
method: "POST",
1668+
body: "Redirect loop",
1669+
headers: {
1670+
"Content-Type": "text/plain",
1671+
},
1672+
});
1673+
1674+
await assertRejects(
1675+
() =>
1676+
doubleKnock(
1677+
request,
1678+
{
1679+
keyId: rsaPublicKey2.id!,
1680+
privateKey: rsaPrivateKey2,
1681+
},
1682+
),
1683+
Error,
1684+
"Too many redirections",
1685+
);
1686+
assertEquals(requestCount, 21);
1687+
1688+
fetchMock.hardReset();
1689+
});
1690+
1691+
test("doubleKnock() detects redirect loops", async () => {
1692+
fetchMock.spyGlobal();
1693+
1694+
let requestCount = 0;
1695+
fetchMock.post("https://example.com/redirect-loop-a", () => {
1696+
requestCount++;
1697+
return Response.redirect("https://example.com/redirect-loop-b", 302);
1698+
});
1699+
fetchMock.post("https://example.com/redirect-loop-b", () => {
1700+
requestCount++;
1701+
return Response.redirect("https://example.com/redirect-loop-a", 302);
1702+
});
1703+
1704+
const request = new Request("https://example.com/redirect-loop-a", {
1705+
method: "POST",
1706+
body: "Redirect loop",
1707+
headers: {
1708+
"Content-Type": "text/plain",
1709+
},
1710+
});
1711+
1712+
await assertRejects(
1713+
() =>
1714+
doubleKnock(
1715+
request,
1716+
{
1717+
keyId: rsaPublicKey2.id!,
1718+
privateKey: rsaPrivateKey2,
1719+
},
1720+
),
1721+
Error,
1722+
"Redirect loop detected",
1723+
);
1724+
assertEquals(requestCount, 2);
1725+
1726+
fetchMock.hardReset();
1727+
});
1728+
16531729
test("doubleKnock() async specDeterminer test", async () => {
16541730
// Install mock fetch handler
16551731
fetchMock.spyGlobal();

0 commit comments

Comments
 (0)