diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a3720385..e2e552ef8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ Docs: https://docs.openclaw.ai - Security/Discord: add `openclaw security audit` warnings for name/tag-based Discord allowlist entries (DM allowlists, guild/channel `users`, and pairing-store entries), highlighting slug-collision risk while keeping name-based matching supported, and canonicalize resolved Discord allowlist names to IDs at runtime without rewriting config files. Thanks @tdjackey for reporting. - Security/Gateway: block node-role connections when device identity metadata is missing. - Security/Media: enforce inbound media byte limits during download/read across Discord, Telegram, Zalo, Microsoft Teams, and BlueBubbles to prevent oversized payload memory spikes before rejection. 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. - 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. - Security/Browser relay: harden extension relay auth token handling for `/extension` and `/cdp` pathways. - Cron: persist `delivered` state in cron job records so delivery failures remain visible in status and logs. (#19174) Thanks @simonemacario. diff --git a/extensions/msteams/src/attachments.test.ts b/extensions/msteams/src/attachments.test.ts index 36a664714..be7251979 100644 --- a/extensions/msteams/src/attachments.test.ts +++ b/extensions/msteams/src/attachments.test.ts @@ -459,6 +459,88 @@ describe("msteams attachments", () => { expect(media.media).toHaveLength(2); }); + + it("blocks SharePoint redirects to hosts outside allowHosts", async () => { + const { downloadMSTeamsGraphMedia } = await load(); + const shareUrl = "https://contoso.sharepoint.com/site/file"; + const escapedUrl = "https://evil.example/internal.pdf"; + fetchRemoteMediaMock.mockImplementationOnce(async (params) => { + const fetchFn = params.fetchImpl ?? fetch; + let currentUrl = params.url; + for (let i = 0; i < 5; i += 1) { + const res = await fetchFn(currentUrl, { redirect: "manual" }); + if ([301, 302, 303, 307, 308].includes(res.status)) { + const location = res.headers.get("location"); + if (!location) { + throw new Error("redirect missing location"); + } + currentUrl = new URL(location, currentUrl).toString(); + continue; + } + if (!res.ok) { + throw new Error(`HTTP ${res.status}`); + } + return { + buffer: Buffer.from(await res.arrayBuffer()), + contentType: res.headers.get("content-type") ?? undefined, + fileName: params.filePathHint, + }; + } + throw new Error("too many redirects"); + }); + + const fetchMock = vi.fn(async (url: string) => { + if (url.endsWith("/hostedContents")) { + return new Response(JSON.stringify({ value: [] }), { status: 200 }); + } + if (url.endsWith("/attachments")) { + return new Response(JSON.stringify({ value: [] }), { status: 200 }); + } + if (url.endsWith("/messages/123")) { + return new Response( + JSON.stringify({ + attachments: [ + { + id: "ref-1", + contentType: "reference", + contentUrl: shareUrl, + name: "report.pdf", + }, + ], + }), + { status: 200 }, + ); + } + if (url.startsWith("https://graph.microsoft.com/v1.0/shares/")) { + return new Response(null, { + status: 302, + headers: { location: escapedUrl }, + }); + } + if (url === escapedUrl) { + return new Response(Buffer.from("should-not-be-fetched"), { + status: 200, + headers: { "content-type": "application/pdf" }, + }); + } + return new Response("not found", { status: 404 }); + }); + + const media = await downloadMSTeamsGraphMedia({ + messageUrl: "https://graph.microsoft.com/v1.0/chats/19%3Achat/messages/123", + tokenProvider: { getAccessToken: vi.fn(async () => "token") }, + maxBytes: 1024 * 1024, + allowHosts: ["graph.microsoft.com", "contoso.sharepoint.com"], + fetchFn: fetchMock as unknown as typeof fetch, + }); + + expect(media.media).toHaveLength(0); + const calledUrls = fetchMock.mock.calls.map((call) => String(call[0])); + expect( + calledUrls.some((url) => url.startsWith("https://graph.microsoft.com/v1.0/shares/")), + ).toBe(true); + expect(calledUrls).not.toContain(escapedUrl); + }); }); describe("buildMSTeamsMediaPayload", () => { diff --git a/extensions/msteams/src/attachments/graph.ts b/extensions/msteams/src/attachments/graph.ts index 7ac94887d..5303246de 100644 --- a/extensions/msteams/src/attachments/graph.ts +++ b/extensions/msteams/src/attachments/graph.ts @@ -5,6 +5,7 @@ import { GRAPH_ROOT, inferPlaceholder, isRecord, + isUrlAllowed, normalizeContentType, resolveRequestUrl, resolveAllowedHosts, @@ -31,6 +32,25 @@ 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 | undefined { let current: unknown = value; for (const key of keys) { @@ -264,6 +284,9 @@ export async function downloadMSTeamsGraphMedia(params: { try { // SharePoint URLs need to be accessed via Graph shares API const shareUrl = att.contentUrl!; + if (!isUrlAllowed(shareUrl, allowHosts)) { + continue; + } const encodedUrl = Buffer.from(shareUrl).toString("base64url"); const sharesUrl = `${GRAPH_ROOT}/shares/u!${encodedUrl}/driveItem/content`; @@ -274,13 +297,20 @@ export async function downloadMSTeamsGraphMedia(params: { contentTypeHint: "application/octet-stream", preserveFilenames: params.preserveFilenames, fetchImpl: async (input, init) => { + const requestUrl = resolveRequestUrl(input); const headers = new Headers(init?.headers); headers.set("Authorization", `Bearer ${accessToken}`); - return await fetchFn(resolveRequestUrl(input), { + const res = await fetchFn(requestUrl, { ...init, headers, - redirect: "follow", }); + 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);