fix(security): enforce msteams redirect allowlist checks
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
@@ -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 | number>): 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);
|
||||
|
||||
Reference in New Issue
Block a user