← Back to PRs

#11812: fix: MSTeams attachment fetch follows redirects before allowlist checks

by coygeek open 2026-02-08 10:46 View on GitHub →
channel: msteams stale
## Fix Summary The MSTeams attachment downloader performs an initial fetch with default redirect behavior before enforcing redirect-target allowlist checks. An attacker-controlled allowed URL can redirect to an internal or otherwise disallowed host, causing server-side request forgery (SSRF). ## Issue Linkage Fixes #11811 ## Security Snapshot - CVSS v3.1: 8.5 (High) - CVSS v4.0: 8.4 (High) ## Implementation Details ### Files Changed - `extensions/msteams/src/attachments.test.ts` (+94/-4) - `extensions/msteams/src/attachments/download.ts` (+56/-34) ### Technical Analysis The MSTeams attachment downloader performs an initial fetch with default redirect behavior before enforcing redirect-target allowlist checks. An attacker-controlled allowed URL can redirect to an internal or otherwise disallowed host, causing server-side request forgery (SSRF). ## Validation Evidence - Command: `pnpm vitest run extensions/msteams/src/attachments.test.ts` - Status: passed ## Risk and Compatibility non-breaking; no known regression impact ## AI-Assisted Disclosure GPT-5.3-Codex <!-- greptile_comment --> <h2>Greptile Overview</h2> <h3>Greptile Summary</h3> This PR hardens the MSTeams attachment downloader against SSRF via redirects by switching requests to `redirect: "manual"`, validating each redirect hop against the host allowlist, and only applying auth headers on redirect hops whose host is in `authAllowHosts`. Tests were updated and extended to cover allowlisted vs non-allowlisted redirect behavior. The core logic change is isolated to `extensions/msteams/src/attachments/download.ts`, with corresponding unit tests in `extensions/msteams/src/attachments.test.ts`. <h3>Confidence Score: 4/5</h3> - This PR is largely safe to merge and closes the redirect-based SSRF gap, with one redirect-limit logic bug to fix first. - Redirect handling is correctly moved to manual mode and each hop is checked against the allowlist; auth headers are also constrained to auth-allowlisted hosts. However, `fetchWithAllowlistedRedirects` currently allows one more redirect hop than intended due to an off-by-one loop condition, which weakens the redirect cap and should be corrected. - extensions/msteams/src/attachments/download.ts <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs