← Back to PRs

#23598: fix(msteams): add SSRF protection to attachment downloads via redirect and DNS validation

by lewiswigmore open 2026-02-22 13:43 View on GitHub →
channel: msteams size: M
## Summary Fixes #11811 The attachment download flow in `fetchWithAuthFallback()` followed redirects automatically on the initial `fetch()` call without any allowlist or IP validation. The hostname allowlist in `isUrlAllowed()` only checked domain names — it never resolved DNS to verify the target IP wasn't in a private range. This enabled two SSRF attack vectors: ### Attack vector 1: Redirect to non-allowlisted host 1. Attacker sends a Teams message with an attachment whose `contentUrl` points to an allowlisted domain 2. That domain returns a 302 redirect to `http://169.254.169.254/metadata` or an internal service 3. The initial `fetch()` call auto-follows the redirect, fetching internal metadata ### Attack vector 2: DNS rebinding via allowlisted domain 1. Attacker controls a subdomain under an allowlisted domain (e.g. `evil.trafficmanager.net`, `evil.azureedge.net`, `evil.blob.core.windows.net`) 2. The subdomain's DNS initially resolves to a public IP (passes the hostname allowlist) 3. On subsequent resolution (or via redirect), it resolves to `169.254.169.254` or `10.x.x.x` 4. The bot fetches internal resources using the attacker's credentials ### Fix Three layers of SSRF protection: 1. **`safeFetch()`** — 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()`** — rejects RFC 1918 (`10.0.0.0/8`, `172.16.0.0/12`, `192.168.0.0/16`), loopback (`127.0.0.0/8`), link-local (`169.254.0.0/16`), and IPv6 private ranges (`::1`, `fe80::/10`, `fc00::/7`) for both initial URLs and redirect targets 3. **`graph.ts` SharePoint redirect handling** — now 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 auto-followed without validation. ### Testing 38 new tests in `shared.test.ts` covering: - IPv4/IPv6 private IP detection (23 cases) - DNS resolution validation (6 cases) - `safeFetch` redirect handling: allowlisted redirects, blocked redirects, DNS rebinding attacks, initial URL IP validation, multi-hop redirects, redirect loops, HTTP downgrade blocking (9 cases) All 192 existing msteams tests continue to pass with zero regressions. ### Files changed - `extensions/msteams/src/attachments/shared.ts` — added `isPrivateOrReservedIP()`, `resolveAndValidateIP()`, `safeFetch()` - `extensions/msteams/src/attachments/download.ts` — `fetchWithAuthFallback()` now uses `safeFetch()` for initial request; auth retry redirects also validated via `resolveAndValidateIP()` - `extensions/msteams/src/attachments/graph.ts` — SharePoint redirect handling uses `redirect: "manual"` and validates resolved IPs - `extensions/msteams/src/attachments/shared.test.ts` — 38 new tests (new file) - `extensions/msteams/src/attachments.test.ts` — existing tests updated to provide mock DNS resolver <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR adds three layers of SSRF protection to the MS Teams attachment download flow: a `safeFetch()` wrapper with manual redirect handling, DNS-to-IP validation via `resolveAndValidateIP()`, and private IP detection via `isPrivateOrReservedIP()`. The approach is sound in principle and the `safeFetch` function itself is well-designed. However, the implementation has gaps that leave two SSRF vectors partially open: - **Chained redirect bypass in `download.ts` and `graph.ts`**: Both files validate the first redirect hop but then follow it with a bare `fetch()` call (no `redirect: "manual"`), allowing the validated redirect target to issue a second redirect to an arbitrary internal URL that is auto-followed without any SSRF checks. - **IPv4-mapped IPv6 bypass in `isPrivateOrReservedIP()`**: The IP check uses string prefix matching, which is bypassed by alternate representations like `::ffff:127.0.0.1` (IPv4-mapped IPv6) and `0:0:0:0:0:0:0:1` (expanded loopback). The codebase already has a robust `isPrivateIpAddress` function in `src/infra/net/ssrf.ts` (exported via the plugin SDK) that handles all these cases — consider reusing it. - **Test coverage**: The 38 new tests in `shared.test.ts` thoroughly cover `safeFetch` and the happy paths, but don't cover the IPv4-mapped IPv6 bypass cases or the chained redirect scenario through auth retry / SharePoint paths. <h3>Confidence Score: 2/5</h3> - This PR partially mitigates SSRF but introduces new code paths that still allow chained redirect attacks and has IP validation bypasses via IPv6 edge cases. - Score of 2 reflects that while the PR's intent and overall architecture are correct (safeFetch, manual redirects, DNS validation), two concrete attack vectors remain: (1) chained redirects from validated targets in download.ts:145 and graph.ts:320 bypass all SSRF checks, and (2) isPrivateOrReservedIP can be bypassed with IPv4-mapped IPv6 addresses. A robust existing implementation (src/infra/net/ssrf.ts) is already available in the codebase but not used. - Pay close attention to `extensions/msteams/src/attachments/download.ts` (line 145), `extensions/msteams/src/attachments/graph.ts` (line 320), and `extensions/msteams/src/attachments/shared.ts` (isPrivateOrReservedIP function) <sub>Last reviewed commit: 4bffc61</sub> <!-- greptile_other_comments_section --> <sub>(3/5) Reply to the agent's comments like "Can you suggest a fix for this @greptileai?" or ask follow-up questions!</sub> <!-- /greptile_comment -->

Most Similar PRs