mirror of
https://github.com/moltbot/moltbot.git
synced 2026-03-07 22:44:16 +00:00
fix(msteams): add SSRF protection to attachment downloads via redirect and DNS validation (#23598)
* fix(msteams): add SSRF protection to attachment downloads via redirect and DNS validation The attachment download flow in fetchWithAuthFallback() followed redirects automatically on the initial fetch without any allowlist or IP validation. This allowed DNS rebinding attacks where an allowlisted domain (e.g. evil.trafficmanager.net) could redirect or resolve to a private IP like 169.254.169.254, bypassing the hostname allowlist entirely (issue #11811). This commit adds three layers of SSRF protection: 1. safeFetch() in shared.ts: a redirect-safe fetch wrapper that uses redirect: "manual" and validates every redirect hop against the hostname allowlist AND DNS-resolved IP before following it. 2. isPrivateOrReservedIP() + resolveAndValidateIP() in shared.ts: rejects RFC 1918, loopback, link-local, and IPv6 private ranges for both initial URLs and redirect targets. 3. graph.ts SharePoint redirect handling now also uses redirect: "manual" and validates resolved IPs, not just hostnames. The initial fetch in fetchWithAuthFallback now goes through safeFetch instead of a bare fetch(), ensuring redirects are never followed without validation. Includes 38 new tests covering IP validation, DNS resolution checks, redirect following, DNS rebinding attacks, redirect loops, and protocol downgrade blocking. * fix: address review feedback on SSRF protection - Replace hand-rolled isPrivateOrReservedIP with SDK's isPrivateIpAddress which handles IPv4-mapped IPv6, expanded notation, NAT64, 6to4, Teredo, octal IPv4, and fails closed on parse errors - Add redirect: "manual" to auth retry redirect fetch in download.ts to prevent chained redirect attacks bypassing SSRF checks - Add redirect: "manual" to SharePoint redirect fetch in graph.ts to prevent the same chained redirect bypass - Update test expectations for SDK's fail-closed behavior on malformed IPs - Add expanded IPv6 loopback (0:0:0:0:0:0:0:1) test case * fix: type fetchMock as typeof fetch to fix TS tuple index error * msteams: harden attachment auth and graph redirect fetch flow * changelog(msteams): credit redirect-safeFetch hardening contributors --------- Co-authored-by: Vincent Koc <vincentkoc@ieee.org>
This commit is contained in:
@@ -216,6 +216,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Security/Control UI: centralize avatar URL/path validation across gateway/config helpers and enforce a 2 MB max size for local agent avatar files before `/avatar` resolution, reducing oversized-avatar memory risk without changing supported avatar formats.
|
||||
- Security/Control UI avatars: harden `/avatar/:agentId` local avatar serving by rejecting symlink paths and requiring fd-level file identity + size checks before reads. This ships in the next npm release. Thanks @tdjackey for reporting.
|
||||
- Security/MSTeams media: enforce allowlist checks for SharePoint reference attachment URLs and redirect targets during Graph-backed media fetches so redirect chains cannot escape configured media host boundaries. This ships in the next npm release. Thanks @tdjackey for reporting.
|
||||
- Security/MSTeams media: route attachment auth-retry and Graph SharePoint download redirects through shared `safeFetch` so each hop is validated with allowlist + DNS/IP checks across the full redirect chain. (#23598) Thanks @Asm3r96 and @lewiswigmore.
|
||||
- Security/macOS discovery: fail closed for unresolved discovery endpoints by clearing stale remote selection values, use resolved service host only for SSH target derivation, and keep remote URL config aligned with resolved endpoint availability. (#21618) Thanks @bmendonca3.
|
||||
- Chat/Usage/TUI: strip synthetic inbound metadata blocks (including `Conversation info` and trailing `Untrusted context` channel metadata wrappers) from displayed conversation history so internal prompt context no longer leaks into user-visible logs.
|
||||
- CI/Tests: fix TypeScript case-table typing and lint assertion regressions so `pnpm check` passes again after Synology Chat landing. (#23012) Thanks @druide67.
|
||||
|
||||
@@ -2,6 +2,9 @@ import type { PluginRuntime } from "openclaw/plugin-sdk";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { setMSTeamsRuntime } from "./runtime.js";
|
||||
|
||||
/** Mock DNS resolver that always returns a public IP (for anti-SSRF validation in tests). */
|
||||
const publicResolveFn = async () => ({ address: "13.107.136.10" });
|
||||
|
||||
const detectMimeMock = vi.fn(async () => "image/png");
|
||||
const saveMediaBufferMock = vi.fn(async () => ({
|
||||
path: "/tmp/saved.png",
|
||||
@@ -142,9 +145,10 @@ describe("msteams attachments", () => {
|
||||
maxBytes: 1024 * 1024,
|
||||
allowHosts: ["x"],
|
||||
fetchFn: fetchMock as unknown as typeof fetch,
|
||||
resolveFn: publicResolveFn,
|
||||
});
|
||||
|
||||
expect(fetchMock).toHaveBeenCalledWith("https://x/img", undefined);
|
||||
expect(fetchMock).toHaveBeenCalled();
|
||||
expect(saveMediaBufferMock).toHaveBeenCalled();
|
||||
expect(media).toHaveLength(1);
|
||||
expect(media[0]?.path).toBe("/tmp/saved.png");
|
||||
@@ -169,9 +173,10 @@ describe("msteams attachments", () => {
|
||||
maxBytes: 1024 * 1024,
|
||||
allowHosts: ["x"],
|
||||
fetchFn: fetchMock as unknown as typeof fetch,
|
||||
resolveFn: publicResolveFn,
|
||||
});
|
||||
|
||||
expect(fetchMock).toHaveBeenCalledWith("https://x/dl", undefined);
|
||||
expect(fetchMock).toHaveBeenCalled();
|
||||
expect(media).toHaveLength(1);
|
||||
});
|
||||
|
||||
@@ -194,9 +199,10 @@ describe("msteams attachments", () => {
|
||||
maxBytes: 1024 * 1024,
|
||||
allowHosts: ["x"],
|
||||
fetchFn: fetchMock as unknown as typeof fetch,
|
||||
resolveFn: publicResolveFn,
|
||||
});
|
||||
|
||||
expect(fetchMock).toHaveBeenCalledWith("https://x/doc.pdf", undefined);
|
||||
expect(fetchMock).toHaveBeenCalled();
|
||||
expect(media).toHaveLength(1);
|
||||
expect(media[0]?.path).toBe("/tmp/saved.pdf");
|
||||
expect(media[0]?.placeholder).toBe("<media:document>");
|
||||
@@ -221,10 +227,11 @@ describe("msteams attachments", () => {
|
||||
maxBytes: 1024 * 1024,
|
||||
allowHosts: ["x"],
|
||||
fetchFn: fetchMock as unknown as typeof fetch,
|
||||
resolveFn: publicResolveFn,
|
||||
});
|
||||
|
||||
expect(media).toHaveLength(1);
|
||||
expect(fetchMock).toHaveBeenCalledWith("https://x/inline.png", undefined);
|
||||
expect(fetchMock).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("stores inline data:image base64 payloads", async () => {
|
||||
@@ -266,11 +273,11 @@ describe("msteams attachments", () => {
|
||||
allowHosts: ["x"],
|
||||
authAllowHosts: ["x"],
|
||||
fetchFn: fetchMock as unknown as typeof fetch,
|
||||
resolveFn: publicResolveFn,
|
||||
});
|
||||
|
||||
expect(fetchMock).toHaveBeenCalled();
|
||||
expect(media).toHaveLength(1);
|
||||
expect(fetchMock).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("skips auth retries when the host is not in auth allowlist", async () => {
|
||||
@@ -297,10 +304,11 @@ describe("msteams attachments", () => {
|
||||
allowHosts: ["azureedge.net"],
|
||||
authAllowHosts: ["graph.microsoft.com"],
|
||||
fetchFn: fetchMock as unknown as typeof fetch,
|
||||
resolveFn: publicResolveFn,
|
||||
});
|
||||
|
||||
expect(media).toHaveLength(0);
|
||||
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||
expect(fetchMock).toHaveBeenCalled();
|
||||
expect(tokenProvider.getAccessToken).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
|
||||
@@ -10,6 +10,7 @@ import {
|
||||
resolveRequestUrl,
|
||||
resolveAuthAllowedHosts,
|
||||
resolveAllowedHosts,
|
||||
safeFetch,
|
||||
} from "./shared.js";
|
||||
import type {
|
||||
MSTeamsAccessTokenProvider,
|
||||
@@ -91,9 +92,19 @@ async function fetchWithAuthFallback(params: {
|
||||
requestInit?: RequestInit;
|
||||
allowHosts: string[];
|
||||
authAllowHosts: string[];
|
||||
resolveFn?: (hostname: string) => Promise<{ address: string }>;
|
||||
}): Promise<Response> {
|
||||
const fetchFn = params.fetchFn ?? fetch;
|
||||
const firstAttempt = await fetchFn(params.url, params.requestInit);
|
||||
|
||||
// Use safeFetch for the initial attempt — redirect: "manual" with
|
||||
// allowlist + DNS/IP validation on every hop (prevents SSRF via redirect).
|
||||
const firstAttempt = await safeFetch({
|
||||
url: params.url,
|
||||
allowHosts: params.allowHosts,
|
||||
fetchFn,
|
||||
requestInit: params.requestInit,
|
||||
resolveFn: params.resolveFn,
|
||||
});
|
||||
if (firstAttempt.ok) {
|
||||
return firstAttempt;
|
||||
}
|
||||
@@ -113,35 +124,40 @@ async function fetchWithAuthFallback(params: {
|
||||
const token = await params.tokenProvider.getAccessToken(scope);
|
||||
const authHeaders = new Headers(params.requestInit?.headers);
|
||||
authHeaders.set("Authorization", `Bearer ${token}`);
|
||||
const res = await fetchFn(params.url, {
|
||||
...params.requestInit,
|
||||
headers: authHeaders,
|
||||
redirect: "manual",
|
||||
const authAttempt = await safeFetch({
|
||||
url: params.url,
|
||||
allowHosts: params.allowHosts,
|
||||
fetchFn,
|
||||
requestInit: {
|
||||
...params.requestInit,
|
||||
headers: authHeaders,
|
||||
},
|
||||
resolveFn: params.resolveFn,
|
||||
});
|
||||
if (res.ok) {
|
||||
return res;
|
||||
if (authAttempt.ok) {
|
||||
return authAttempt;
|
||||
}
|
||||
const redirectUrl = readRedirectUrl(params.url, res);
|
||||
if (redirectUrl && isUrlAllowed(redirectUrl, params.allowHosts)) {
|
||||
const redirectRes = await fetchFn(redirectUrl, params.requestInit);
|
||||
if (redirectRes.ok) {
|
||||
return redirectRes;
|
||||
}
|
||||
if (
|
||||
(redirectRes.status === 401 || redirectRes.status === 403) &&
|
||||
isUrlAllowed(redirectUrl, params.authAllowHosts)
|
||||
) {
|
||||
const redirectAuthHeaders = new Headers(params.requestInit?.headers);
|
||||
redirectAuthHeaders.set("Authorization", `Bearer ${token}`);
|
||||
const redirectAuthRes = await fetchFn(redirectUrl, {
|
||||
...params.requestInit,
|
||||
headers: redirectAuthHeaders,
|
||||
redirect: "manual",
|
||||
});
|
||||
if (redirectAuthRes.ok) {
|
||||
return redirectAuthRes;
|
||||
}
|
||||
}
|
||||
if (authAttempt.status !== 401 && authAttempt.status !== 403) {
|
||||
continue;
|
||||
}
|
||||
|
||||
const finalUrl =
|
||||
typeof authAttempt.url === "string" && authAttempt.url ? authAttempt.url : "";
|
||||
if (!finalUrl || finalUrl === params.url || !isUrlAllowed(finalUrl, params.authAllowHosts)) {
|
||||
continue;
|
||||
}
|
||||
const redirectedAuthAttempt = await safeFetch({
|
||||
url: finalUrl,
|
||||
allowHosts: params.allowHosts,
|
||||
fetchFn,
|
||||
requestInit: {
|
||||
...params.requestInit,
|
||||
headers: authHeaders,
|
||||
},
|
||||
resolveFn: params.resolveFn,
|
||||
});
|
||||
if (redirectedAuthAttempt.ok) {
|
||||
return redirectedAuthAttempt;
|
||||
}
|
||||
} catch {
|
||||
// Try the next scope.
|
||||
@@ -151,21 +167,6 @@ async function fetchWithAuthFallback(params: {
|
||||
return firstAttempt;
|
||||
}
|
||||
|
||||
function readRedirectUrl(baseUrl: string, res: Response): string | null {
|
||||
if (![301, 302, 303, 307, 308].includes(res.status)) {
|
||||
return null;
|
||||
}
|
||||
const location = res.headers.get("location");
|
||||
if (!location) {
|
||||
return null;
|
||||
}
|
||||
try {
|
||||
return new URL(location, baseUrl).toString();
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Download all file attachments from a Teams message (images, documents, etc.).
|
||||
* Renamed from downloadMSTeamsImageAttachments to support all file types.
|
||||
@@ -179,6 +180,8 @@ export async function downloadMSTeamsAttachments(params: {
|
||||
fetchFn?: typeof fetch;
|
||||
/** When true, embeds original filename in stored path for later extraction. */
|
||||
preserveFilenames?: boolean;
|
||||
/** Override DNS resolver for testing (anti-SSRF IP validation). */
|
||||
resolveFn?: (hostname: string) => Promise<{ address: string }>;
|
||||
}): Promise<MSTeamsInboundMedia[]> {
|
||||
const list = Array.isArray(params.attachments) ? params.attachments : [];
|
||||
if (list.length === 0) {
|
||||
@@ -262,6 +265,7 @@ export async function downloadMSTeamsAttachments(params: {
|
||||
requestInit: init,
|
||||
allowHosts,
|
||||
authAllowHosts,
|
||||
resolveFn: params.resolveFn,
|
||||
}),
|
||||
});
|
||||
out.push(media);
|
||||
|
||||
@@ -9,6 +9,7 @@ import {
|
||||
normalizeContentType,
|
||||
resolveRequestUrl,
|
||||
resolveAllowedHosts,
|
||||
safeFetch,
|
||||
} from "./shared.js";
|
||||
import type {
|
||||
MSTeamsAccessTokenProvider,
|
||||
@@ -32,25 +33,6 @@ type GraphAttachment = {
|
||||
content?: unknown;
|
||||
};
|
||||
|
||||
function isRedirectStatus(status: number): boolean {
|
||||
return [301, 302, 303, 307, 308].includes(status);
|
||||
}
|
||||
|
||||
function readRedirectUrl(baseUrl: string, res: Response): string | null {
|
||||
if (!isRedirectStatus(res.status)) {
|
||||
return null;
|
||||
}
|
||||
const location = res.headers.get("location");
|
||||
if (!location) {
|
||||
return null;
|
||||
}
|
||||
try {
|
||||
return new URL(location, baseUrl).toString();
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
function readNestedString(value: unknown, keys: Array<string | number>): string | undefined {
|
||||
let current: unknown = value;
|
||||
for (const key of keys) {
|
||||
@@ -300,17 +282,15 @@ export async function downloadMSTeamsGraphMedia(params: {
|
||||
const requestUrl = resolveRequestUrl(input);
|
||||
const headers = new Headers(init?.headers);
|
||||
headers.set("Authorization", `Bearer ${accessToken}`);
|
||||
const res = await fetchFn(requestUrl, {
|
||||
...init,
|
||||
headers,
|
||||
return await safeFetch({
|
||||
url: requestUrl,
|
||||
allowHosts,
|
||||
fetchFn,
|
||||
requestInit: {
|
||||
...init,
|
||||
headers,
|
||||
},
|
||||
});
|
||||
const redirectUrl = readRedirectUrl(requestUrl, res);
|
||||
if (redirectUrl && !isUrlAllowed(redirectUrl, allowHosts)) {
|
||||
throw new Error(
|
||||
`MSTeams media redirect target blocked by allowlist: ${redirectUrl}`,
|
||||
);
|
||||
}
|
||||
return res;
|
||||
},
|
||||
});
|
||||
sharePointMedia.push(media);
|
||||
|
||||
279
extensions/msteams/src/attachments/shared.test.ts
Normal file
279
extensions/msteams/src/attachments/shared.test.ts
Normal file
@@ -0,0 +1,279 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { isPrivateOrReservedIP, resolveAndValidateIP, safeFetch } from "./shared.js";
|
||||
|
||||
// ─── Helpers ─────────────────────────────────────────────────────────────────
|
||||
|
||||
const publicResolve = async () => ({ address: "13.107.136.10" });
|
||||
const privateResolve = (ip: string) => async () => ({ address: ip });
|
||||
const failingResolve = async () => {
|
||||
throw new Error("DNS failure");
|
||||
};
|
||||
|
||||
function mockFetchWithRedirect(redirectMap: Record<string, string>, finalBody = "ok") {
|
||||
return vi.fn(async (url: string, init?: RequestInit) => {
|
||||
const target = redirectMap[url];
|
||||
if (target && init?.redirect === "manual") {
|
||||
return new Response(null, {
|
||||
status: 302,
|
||||
headers: { location: target },
|
||||
});
|
||||
}
|
||||
return new Response(finalBody, { status: 200 });
|
||||
});
|
||||
}
|
||||
|
||||
// ─── isPrivateOrReservedIP ───────────────────────────────────────────────────
|
||||
|
||||
describe("isPrivateOrReservedIP", () => {
|
||||
it.each([
|
||||
["10.0.0.1", true],
|
||||
["10.255.255.255", true],
|
||||
["172.16.0.1", true],
|
||||
["172.31.255.255", true],
|
||||
["172.15.0.1", false],
|
||||
["172.32.0.1", false],
|
||||
["192.168.0.1", true],
|
||||
["192.168.255.255", true],
|
||||
["127.0.0.1", true],
|
||||
["127.255.255.255", true],
|
||||
["169.254.0.1", true],
|
||||
["169.254.169.254", true],
|
||||
["0.0.0.0", true],
|
||||
["8.8.8.8", false],
|
||||
["13.107.136.10", false],
|
||||
["52.96.0.1", false],
|
||||
] as const)("IPv4 %s → %s", (ip, expected) => {
|
||||
expect(isPrivateOrReservedIP(ip)).toBe(expected);
|
||||
});
|
||||
|
||||
it.each([
|
||||
["::1", true],
|
||||
["::", true],
|
||||
["fe80::1", true],
|
||||
["fc00::1", true],
|
||||
["fd12:3456::1", true],
|
||||
["2001:0db8::1", false],
|
||||
["2620:1ec:c11::200", false],
|
||||
// IPv4-mapped IPv6 addresses
|
||||
["::ffff:127.0.0.1", true],
|
||||
["::ffff:10.0.0.1", true],
|
||||
["::ffff:192.168.1.1", true],
|
||||
["::ffff:169.254.169.254", true],
|
||||
["::ffff:8.8.8.8", false],
|
||||
["::ffff:13.107.136.10", false],
|
||||
] as const)("IPv6 %s → %s", (ip, expected) => {
|
||||
expect(isPrivateOrReservedIP(ip)).toBe(expected);
|
||||
});
|
||||
|
||||
it.each([
|
||||
["999.999.999.999", true],
|
||||
["256.0.0.1", true],
|
||||
["10.0.0.256", true],
|
||||
["-1.0.0.1", false],
|
||||
["1.2.3.4.5", false],
|
||||
["0:0:0:0:0:0:0:1", true],
|
||||
] as const)("malformed/expanded %s → %s (SDK fails closed)", (ip, expected) => {
|
||||
expect(isPrivateOrReservedIP(ip)).toBe(expected);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── resolveAndValidateIP ────────────────────────────────────────────────────
|
||||
|
||||
describe("resolveAndValidateIP", () => {
|
||||
it("accepts a hostname resolving to a public IP", async () => {
|
||||
const ip = await resolveAndValidateIP("teams.sharepoint.com", publicResolve);
|
||||
expect(ip).toBe("13.107.136.10");
|
||||
});
|
||||
|
||||
it("rejects a hostname resolving to 10.x.x.x", async () => {
|
||||
await expect(resolveAndValidateIP("evil.test", privateResolve("10.0.0.1"))).rejects.toThrow(
|
||||
"private/reserved IP",
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects a hostname resolving to 169.254.169.254", async () => {
|
||||
await expect(
|
||||
resolveAndValidateIP("evil.test", privateResolve("169.254.169.254")),
|
||||
).rejects.toThrow("private/reserved IP");
|
||||
});
|
||||
|
||||
it("rejects a hostname resolving to loopback", async () => {
|
||||
await expect(resolveAndValidateIP("evil.test", privateResolve("127.0.0.1"))).rejects.toThrow(
|
||||
"private/reserved IP",
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects a hostname resolving to IPv6 loopback", async () => {
|
||||
await expect(resolveAndValidateIP("evil.test", privateResolve("::1"))).rejects.toThrow(
|
||||
"private/reserved IP",
|
||||
);
|
||||
});
|
||||
|
||||
it("throws on DNS resolution failure", async () => {
|
||||
await expect(resolveAndValidateIP("nonexistent.test", failingResolve)).rejects.toThrow(
|
||||
"DNS resolution failed",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── safeFetch ───────────────────────────────────────────────────────────────
|
||||
|
||||
describe("safeFetch", () => {
|
||||
it("fetches a URL directly when no redirect occurs", async () => {
|
||||
const fetchMock = vi.fn<typeof fetch>(async () => new Response("ok", { status: 200 }));
|
||||
const res = await safeFetch({
|
||||
url: "https://teams.sharepoint.com/file.pdf",
|
||||
allowHosts: ["sharepoint.com"],
|
||||
fetchFn: fetchMock,
|
||||
resolveFn: publicResolve,
|
||||
});
|
||||
expect(res.status).toBe(200);
|
||||
expect(fetchMock).toHaveBeenCalledOnce();
|
||||
// Should have used redirect: "manual"
|
||||
expect(fetchMock.mock.calls[0][1]).toHaveProperty("redirect", "manual");
|
||||
});
|
||||
|
||||
it("follows a redirect to an allowlisted host with public IP", async () => {
|
||||
const fetchMock = mockFetchWithRedirect({
|
||||
"https://teams.sharepoint.com/file.pdf": "https://cdn.sharepoint.com/storage/file.pdf",
|
||||
});
|
||||
const res = await safeFetch({
|
||||
url: "https://teams.sharepoint.com/file.pdf",
|
||||
allowHosts: ["sharepoint.com"],
|
||||
fetchFn: fetchMock as unknown as typeof fetch,
|
||||
resolveFn: publicResolve,
|
||||
});
|
||||
expect(res.status).toBe(200);
|
||||
expect(fetchMock).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("blocks a redirect to a non-allowlisted host", async () => {
|
||||
const fetchMock = mockFetchWithRedirect({
|
||||
"https://teams.sharepoint.com/file.pdf": "https://evil.example.com/steal",
|
||||
});
|
||||
await expect(
|
||||
safeFetch({
|
||||
url: "https://teams.sharepoint.com/file.pdf",
|
||||
allowHosts: ["sharepoint.com"],
|
||||
fetchFn: fetchMock as unknown as typeof fetch,
|
||||
resolveFn: publicResolve,
|
||||
}),
|
||||
).rejects.toThrow("blocked by allowlist");
|
||||
// Should not have fetched the evil URL
|
||||
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("blocks a redirect to an allowlisted host that resolves to a private IP (DNS rebinding)", async () => {
|
||||
let callCount = 0;
|
||||
const rebindingResolve = async () => {
|
||||
callCount++;
|
||||
// First call (initial URL) resolves to public IP
|
||||
if (callCount === 1) return { address: "13.107.136.10" };
|
||||
// Second call (redirect target) resolves to private IP
|
||||
return { address: "169.254.169.254" };
|
||||
};
|
||||
|
||||
const fetchMock = mockFetchWithRedirect({
|
||||
"https://teams.sharepoint.com/file.pdf": "https://evil.trafficmanager.net/metadata",
|
||||
});
|
||||
await expect(
|
||||
safeFetch({
|
||||
url: "https://teams.sharepoint.com/file.pdf",
|
||||
allowHosts: ["sharepoint.com", "trafficmanager.net"],
|
||||
fetchFn: fetchMock as unknown as typeof fetch,
|
||||
resolveFn: rebindingResolve,
|
||||
}),
|
||||
).rejects.toThrow("private/reserved IP");
|
||||
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("blocks when the initial URL resolves to a private IP", async () => {
|
||||
const fetchMock = vi.fn();
|
||||
await expect(
|
||||
safeFetch({
|
||||
url: "https://evil.sharepoint.com/file.pdf",
|
||||
allowHosts: ["sharepoint.com"],
|
||||
fetchFn: fetchMock as unknown as typeof fetch,
|
||||
resolveFn: privateResolve("10.0.0.1"),
|
||||
}),
|
||||
).rejects.toThrow("Initial download URL blocked");
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("blocks when initial URL DNS resolution fails", async () => {
|
||||
const fetchMock = vi.fn();
|
||||
await expect(
|
||||
safeFetch({
|
||||
url: "https://nonexistent.sharepoint.com/file.pdf",
|
||||
allowHosts: ["sharepoint.com"],
|
||||
fetchFn: fetchMock as unknown as typeof fetch,
|
||||
resolveFn: failingResolve,
|
||||
}),
|
||||
).rejects.toThrow("Initial download URL blocked");
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("follows multiple redirects when all are valid", async () => {
|
||||
const fetchMock = vi.fn(async (url: string, init?: RequestInit) => {
|
||||
if (url === "https://a.sharepoint.com/1" && init?.redirect === "manual") {
|
||||
return new Response(null, {
|
||||
status: 302,
|
||||
headers: { location: "https://b.sharepoint.com/2" },
|
||||
});
|
||||
}
|
||||
if (url === "https://b.sharepoint.com/2" && init?.redirect === "manual") {
|
||||
return new Response(null, {
|
||||
status: 302,
|
||||
headers: { location: "https://c.sharepoint.com/3" },
|
||||
});
|
||||
}
|
||||
return new Response("final", { status: 200 });
|
||||
});
|
||||
|
||||
const res = await safeFetch({
|
||||
url: "https://a.sharepoint.com/1",
|
||||
allowHosts: ["sharepoint.com"],
|
||||
fetchFn: fetchMock as unknown as typeof fetch,
|
||||
resolveFn: publicResolve,
|
||||
});
|
||||
expect(res.status).toBe(200);
|
||||
expect(fetchMock).toHaveBeenCalledTimes(3);
|
||||
});
|
||||
|
||||
it("throws on too many redirects", async () => {
|
||||
let counter = 0;
|
||||
const fetchMock = vi.fn(async (_url: string, init?: RequestInit) => {
|
||||
if (init?.redirect === "manual") {
|
||||
counter++;
|
||||
return new Response(null, {
|
||||
status: 302,
|
||||
headers: { location: `https://loop${counter}.sharepoint.com/x` },
|
||||
});
|
||||
}
|
||||
return new Response("ok", { status: 200 });
|
||||
});
|
||||
|
||||
await expect(
|
||||
safeFetch({
|
||||
url: "https://start.sharepoint.com/x",
|
||||
allowHosts: ["sharepoint.com"],
|
||||
fetchFn: fetchMock as unknown as typeof fetch,
|
||||
resolveFn: publicResolve,
|
||||
}),
|
||||
).rejects.toThrow("Too many redirects");
|
||||
});
|
||||
|
||||
it("blocks redirect to HTTP (non-HTTPS)", async () => {
|
||||
const fetchMock = mockFetchWithRedirect({
|
||||
"https://teams.sharepoint.com/file": "http://internal.sharepoint.com/file",
|
||||
});
|
||||
await expect(
|
||||
safeFetch({
|
||||
url: "https://teams.sharepoint.com/file",
|
||||
allowHosts: ["sharepoint.com"],
|
||||
fetchFn: fetchMock as unknown as typeof fetch,
|
||||
resolveFn: publicResolve,
|
||||
}),
|
||||
).rejects.toThrow("blocked by allowlist");
|
||||
});
|
||||
});
|
||||
@@ -1,3 +1,5 @@
|
||||
import { lookup } from "node:dns/promises";
|
||||
import { isPrivateIpAddress } from "openclaw/plugin-sdk";
|
||||
import type { MSTeamsAttachmentLike } from "./types.js";
|
||||
|
||||
type InlineImageCandidate =
|
||||
@@ -302,3 +304,101 @@ export function isUrlAllowed(url: string, allowlist: string[]): boolean {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns true if the given IPv4 or IPv6 address is in a private, loopback,
|
||||
* or link-local range that must never be reached from media downloads.
|
||||
*
|
||||
* Delegates to the SDK's `isPrivateIpAddress` which handles IPv4-mapped IPv6,
|
||||
* expanded notation, NAT64, 6to4, Teredo, octal IPv4, and fails closed on
|
||||
* parse errors.
|
||||
*/
|
||||
export const isPrivateOrReservedIP: (ip: string) => boolean = isPrivateIpAddress;
|
||||
|
||||
/**
|
||||
* Resolve a hostname via DNS and reject private/reserved IPs.
|
||||
* Throws if the resolved IP is private or resolution fails.
|
||||
*/
|
||||
export async function resolveAndValidateIP(
|
||||
hostname: string,
|
||||
resolveFn?: (hostname: string) => Promise<{ address: string }>,
|
||||
): Promise<string> {
|
||||
const resolve = resolveFn ?? lookup;
|
||||
let resolved: { address: string };
|
||||
try {
|
||||
resolved = await resolve(hostname);
|
||||
} catch {
|
||||
throw new Error(`DNS resolution failed for "${hostname}"`);
|
||||
}
|
||||
if (isPrivateOrReservedIP(resolved.address)) {
|
||||
throw new Error(`Hostname "${hostname}" resolves to private/reserved IP (${resolved.address})`);
|
||||
}
|
||||
return resolved.address;
|
||||
}
|
||||
|
||||
/** Maximum number of redirects to follow in safeFetch. */
|
||||
const MAX_SAFE_REDIRECTS = 5;
|
||||
|
||||
/**
|
||||
* Fetch a URL with redirect: "manual", validating each redirect target
|
||||
* against the hostname allowlist and DNS-resolved IP (anti-SSRF).
|
||||
*
|
||||
* This prevents:
|
||||
* - Auto-following redirects to non-allowlisted hosts
|
||||
* - DNS rebinding attacks where an allowlisted domain resolves to a private IP
|
||||
*/
|
||||
export async function safeFetch(params: {
|
||||
url: string;
|
||||
allowHosts: string[];
|
||||
fetchFn?: typeof fetch;
|
||||
requestInit?: RequestInit;
|
||||
resolveFn?: (hostname: string) => Promise<{ address: string }>;
|
||||
}): Promise<Response> {
|
||||
const fetchFn = params.fetchFn ?? fetch;
|
||||
const resolveFn = params.resolveFn;
|
||||
let currentUrl = params.url;
|
||||
|
||||
// Validate the initial URL's resolved IP
|
||||
try {
|
||||
const initialHost = new URL(currentUrl).hostname;
|
||||
await resolveAndValidateIP(initialHost, resolveFn);
|
||||
} catch {
|
||||
throw new Error(`Initial download URL blocked: ${currentUrl}`);
|
||||
}
|
||||
|
||||
for (let i = 0; i <= MAX_SAFE_REDIRECTS; i++) {
|
||||
const res = await fetchFn(currentUrl, {
|
||||
...params.requestInit,
|
||||
redirect: "manual",
|
||||
});
|
||||
|
||||
if (![301, 302, 303, 307, 308].includes(res.status)) {
|
||||
return res;
|
||||
}
|
||||
|
||||
const location = res.headers.get("location");
|
||||
if (!location) {
|
||||
return res;
|
||||
}
|
||||
|
||||
let redirectUrl: string;
|
||||
try {
|
||||
redirectUrl = new URL(location, currentUrl).toString();
|
||||
} catch {
|
||||
throw new Error(`Invalid redirect URL: ${location}`);
|
||||
}
|
||||
|
||||
// Validate redirect target against hostname allowlist
|
||||
if (!isUrlAllowed(redirectUrl, params.allowHosts)) {
|
||||
throw new Error(`Media redirect target blocked by allowlist: ${redirectUrl}`);
|
||||
}
|
||||
|
||||
// Validate redirect target's resolved IP
|
||||
const redirectHost = new URL(redirectUrl).hostname;
|
||||
await resolveAndValidateIP(redirectHost, resolveFn);
|
||||
|
||||
currentUrl = redirectUrl;
|
||||
}
|
||||
|
||||
throw new Error(`Too many redirects (>${MAX_SAFE_REDIRECTS})`);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user