mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-12 07:20:45 +00:00
msteams: enforce guarded redirect ownership in safeFetch
This commit is contained in:
committed by
Peter Steinberger
parent
00347bda75
commit
cceecc8bd4
@@ -694,6 +694,54 @@ describe("msteams attachments", () => {
|
||||
runAttachmentAuthRetryCase,
|
||||
);
|
||||
|
||||
it("preserves auth fallback when dispatcher-mode fetch returns a redirect", async () => {
|
||||
const redirectedUrl = createTestUrl("redirected.png");
|
||||
const tokenProvider = createTokenProvider();
|
||||
const fetchMock = vi.fn(async (url: string, opts?: RequestInit) => {
|
||||
const hasAuth = Boolean(new Headers(opts?.headers).get("Authorization"));
|
||||
if (url === TEST_URL_IMAGE) {
|
||||
return hasAuth
|
||||
? createRedirectResponse(redirectedUrl)
|
||||
: createTextResponse("unauthorized", 401);
|
||||
}
|
||||
if (url === redirectedUrl) {
|
||||
return createBufferResponse(PNG_BUFFER, CONTENT_TYPE_IMAGE_PNG);
|
||||
}
|
||||
return createNotFoundResponse();
|
||||
});
|
||||
|
||||
fetchRemoteMediaMock.mockImplementationOnce(async (params) => {
|
||||
const fetchFn = params.fetchImpl ?? fetch;
|
||||
let currentUrl = params.url;
|
||||
for (let i = 0; i < MAX_REDIRECT_HOPS; i += 1) {
|
||||
const res = await fetchFn(currentUrl, {
|
||||
redirect: "manual",
|
||||
dispatcher: {},
|
||||
} as RequestInit);
|
||||
if (REDIRECT_STATUS_CODES.includes(res.status)) {
|
||||
const location = res.headers.get("location");
|
||||
if (!location) {
|
||||
throw new Error("redirect missing location");
|
||||
}
|
||||
currentUrl = new URL(location, currentUrl).toString();
|
||||
continue;
|
||||
}
|
||||
return readRemoteMediaResponse(res, params);
|
||||
}
|
||||
throw new Error("too many redirects");
|
||||
});
|
||||
|
||||
const media = await downloadAttachmentsWithFetch(
|
||||
createImageAttachments(TEST_URL_IMAGE),
|
||||
fetchMock,
|
||||
{ tokenProvider, authAllowHosts: [TEST_HOST] },
|
||||
);
|
||||
|
||||
expectAttachmentMediaLength(media, 1);
|
||||
expect(tokenProvider.getAccessToken).toHaveBeenCalledOnce();
|
||||
expect(fetchMock.mock.calls.map(([calledUrl]) => String(calledUrl))).toContain(redirectedUrl);
|
||||
});
|
||||
|
||||
it("skips urls outside the allowlist", async () => {
|
||||
const fetchMock = vi.fn();
|
||||
const media = await downloadAttachmentsWithFetch(
|
||||
|
||||
@@ -102,6 +102,68 @@ async function fetchWithAuthFallback(params: {
|
||||
requireHttps: true,
|
||||
shouldAttachAuth: (url) => isUrlAllowed(url, params.authAllowHosts),
|
||||
});
|
||||
if (firstAttempt.ok) {
|
||||
return firstAttempt;
|
||||
}
|
||||
if (!params.tokenProvider) {
|
||||
return firstAttempt;
|
||||
}
|
||||
if (firstAttempt.status !== 401 && firstAttempt.status !== 403) {
|
||||
return firstAttempt;
|
||||
}
|
||||
if (!isUrlAllowed(params.url, params.authAllowHosts)) {
|
||||
return firstAttempt;
|
||||
}
|
||||
|
||||
const scopes = scopeCandidatesForUrl(params.url);
|
||||
for (const scope of scopes) {
|
||||
try {
|
||||
const token = await params.tokenProvider.getAccessToken(scope);
|
||||
const authHeaders = new Headers(params.requestInit?.headers);
|
||||
authHeaders.set("Authorization", `Bearer ${token}`);
|
||||
const authAttempt = await safeFetch({
|
||||
url: params.url,
|
||||
allowHosts: params.allowHosts,
|
||||
fetchFn,
|
||||
requestInit: {
|
||||
...params.requestInit,
|
||||
headers: authHeaders,
|
||||
},
|
||||
resolveFn: params.resolveFn,
|
||||
});
|
||||
if (authAttempt.ok) {
|
||||
return authAttempt;
|
||||
}
|
||||
if (authAttempt.status !== 401 && authAttempt.status !== 403) {
|
||||
// Non-auth failures (including redirects in guarded fetch mode) should
|
||||
// be handled by the caller's redirect/error policy.
|
||||
return authAttempt;
|
||||
}
|
||||
|
||||
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.
|
||||
}
|
||||
}
|
||||
|
||||
return firstAttempt;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -19,10 +19,250 @@ describe("msteams attachment allowlists", () => {
|
||||
expect(isUrlAllowed("https://evil.example.com/file.png", allowHosts)).toBe(false);
|
||||
});
|
||||
|
||||
it("builds shared SSRF policy from suffix allowlist", () => {
|
||||
expect(resolveMediaSsrfPolicy(["sharepoint.com"])).toEqual({
|
||||
hostnameAllowlist: ["sharepoint.com", "*.sharepoint.com"],
|
||||
});
|
||||
expect(resolveMediaSsrfPolicy(["*"])).toBeUndefined();
|
||||
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(async (_url: string, _init?: RequestInit) => {
|
||||
return new Response("ok", { status: 200 });
|
||||
});
|
||||
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).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("returns the redirect response when dispatcher is provided by an outer guard", async () => {
|
||||
const redirectedTo = "https://cdn.sharepoint.com/storage/file.pdf";
|
||||
const fetchMock = mockFetchWithRedirect({
|
||||
"https://teams.sharepoint.com/file.pdf": redirectedTo,
|
||||
});
|
||||
const res = await safeFetch({
|
||||
url: "https://teams.sharepoint.com/file.pdf",
|
||||
allowHosts: ["sharepoint.com"],
|
||||
fetchFn: fetchMock as unknown as typeof fetch,
|
||||
requestInit: { dispatcher: {} } as RequestInit,
|
||||
resolveFn: publicResolve,
|
||||
});
|
||||
expect(res.status).toBe(302);
|
||||
expect(res.headers.get("location")).toBe(redirectedTo);
|
||||
expect(fetchMock).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it("still enforces allowlist checks before returning dispatcher-mode redirects", 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,
|
||||
requestInit: { dispatcher: {} } as RequestInit,
|
||||
resolveFn: publicResolve,
|
||||
}),
|
||||
).rejects.toThrow("blocked by allowlist");
|
||||
expect(fetchMock).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
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");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -268,6 +268,112 @@ export function isUrlAllowed(url: string, allowlist: string[]): boolean {
|
||||
return isHttpsUrlAllowedByHostnameSuffixAllowlist(url, allowlist);
|
||||
}
|
||||
|
||||
export function resolveMediaSsrfPolicy(allowHosts: string[]): SsrFPolicy | undefined {
|
||||
return buildHostnameAllowlistPolicyFromSuffixAllowlist(allowHosts);
|
||||
/**
|
||||
* 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;
|
||||
const hasDispatcher = Boolean(
|
||||
params.requestInit &&
|
||||
typeof params.requestInit === "object" &&
|
||||
"dispatcher" in (params.requestInit as Record<string, unknown>),
|
||||
);
|
||||
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}`);
|
||||
}
|
||||
|
||||
// When a pinned dispatcher is already injected by an upstream guard
|
||||
// (for example fetchWithSsrFGuard), let that guard own redirect handling
|
||||
// after this allowlist validation step.
|
||||
if (hasDispatcher) {
|
||||
return res;
|
||||
}
|
||||
|
||||
// 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