fix(msteams): add SSRF validation to file consent upload URL#23596
Merged
BradGroux merged 3 commits intoopenclaw:mainfrom Apr 6, 2026
Merged
fix(msteams): add SSRF validation to file consent upload URL#23596BradGroux merged 3 commits intoopenclaw:mainfrom
BradGroux merged 3 commits intoopenclaw:mainfrom
Conversation
Comment on lines
+57
to
+67
| // IPv6 checks | ||
| const normalized = ip.toLowerCase(); | ||
| // ::1 loopback | ||
| if (normalized === "::1") return true; | ||
| // fe80::/10 link-local | ||
| if (normalized.startsWith("fe80:") || normalized.startsWith("fe80")) return true; | ||
| // fc00::/7 unique-local (fc00:: and fd00::) | ||
| if (normalized.startsWith("fc") || normalized.startsWith("fd")) return true; | ||
| // :: unspecified | ||
| if (normalized === "::") return true; | ||
|
|
Contributor
There was a problem hiding this comment.
IPv6 detection incomplete - doesn't handle IPv4-embedded addresses like ::ffff:127.0.0.1 which bypass validation
Suggested change
| // IPv6 checks | |
| const normalized = ip.toLowerCase(); | |
| // ::1 loopback | |
| if (normalized === "::1") return true; | |
| // fe80::/10 link-local | |
| if (normalized.startsWith("fe80:") || normalized.startsWith("fe80")) return true; | |
| // fc00::/7 unique-local (fc00:: and fd00::) | |
| if (normalized.startsWith("fc") || normalized.startsWith("fd")) return true; | |
| // :: unspecified | |
| if (normalized === "::") return true; | |
| // IPv6 checks | |
| const normalized = ip.toLowerCase(); | |
| // Handle IPv4-embedded IPv6 (e.g., ::ffff:127.0.0.1, ::ffff:10.0.0.1) | |
| const ipv4MappedMatch = /::ffff:(\d+\.\d+\.\d+\.\d+)$/i.exec(normalized); | |
| if (ipv4MappedMatch) { | |
| return isPrivateOrReservedIP(ipv4MappedMatch[1]); | |
| } | |
| // ::1 loopback | |
| if (normalized === "::1") return true; | |
| // fe80::/10 link-local | |
| if (normalized.startsWith("fe80:") || normalized.startsWith("fe80")) return true; | |
| // fc00::/7 unique-local (fc00:: and fd00::) | |
| if (normalized.startsWith("fc") || normalized.startsWith("fd")) return true; | |
| // :: unspecified | |
| if (normalized === "::") return true; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/msteams/src/file-consent.ts
Line: 57-67
Comment:
IPv6 detection incomplete - doesn't handle IPv4-embedded addresses like `::ffff:127.0.0.1` which bypass validation
```suggestion
// IPv6 checks
const normalized = ip.toLowerCase();
// Handle IPv4-embedded IPv6 (e.g., ::ffff:127.0.0.1, ::ffff:10.0.0.1)
const ipv4MappedMatch = /::ffff:(\d+\.\d+\.\d+\.\d+)$/i.exec(normalized);
if (ipv4MappedMatch) {
return isPrivateOrReservedIP(ipv4MappedMatch[1]);
}
// ::1 loopback
if (normalized === "::1") return true;
// fe80::/10 link-local
if (normalized.startsWith("fe80:") || normalized.startsWith("fe80")) return true;
// fc00::/7 unique-local (fc00:: and fd00::)
if (normalized.startsWith("fc") || normalized.startsWith("fd")) return true;
// :: unspecified
if (normalized === "::") return true;
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+38
to
+55
| // IPv4 checks | ||
| const v4Parts = ip.split("."); | ||
| if (v4Parts.length === 4) { | ||
| const a = Number(v4Parts[0]); | ||
| const b = Number(v4Parts[1]); | ||
| // 10.0.0.0/8 | ||
| if (a === 10) return true; | ||
| // 172.16.0.0/12 | ||
| if (a === 172 && b >= 16 && b <= 31) return true; | ||
| // 192.168.0.0/16 | ||
| if (a === 192 && b === 168) return true; | ||
| // 127.0.0.0/8 (loopback) | ||
| if (a === 127) return true; | ||
| // 169.254.0.0/16 (link-local) | ||
| if (a === 169 && b === 254) return true; | ||
| // 0.0.0.0/8 | ||
| if (a === 0) return true; | ||
| } |
Contributor
There was a problem hiding this comment.
IPv4 validation doesn't verify octets are 0-255, allowing invalid IPs like 999.999.999.999 to pass through
Suggested change
| // IPv4 checks | |
| const v4Parts = ip.split("."); | |
| if (v4Parts.length === 4) { | |
| const a = Number(v4Parts[0]); | |
| const b = Number(v4Parts[1]); | |
| // 10.0.0.0/8 | |
| if (a === 10) return true; | |
| // 172.16.0.0/12 | |
| if (a === 172 && b >= 16 && b <= 31) return true; | |
| // 192.168.0.0/16 | |
| if (a === 192 && b === 168) return true; | |
| // 127.0.0.0/8 (loopback) | |
| if (a === 127) return true; | |
| // 169.254.0.0/16 (link-local) | |
| if (a === 169 && b === 254) return true; | |
| // 0.0.0.0/8 | |
| if (a === 0) return true; | |
| } | |
| // IPv4 checks | |
| const v4Parts = ip.split("."); | |
| if (v4Parts.length === 4) { | |
| const octets = v4Parts.map(Number); | |
| // Validate all parts are valid 0-255 | |
| if (octets.some((n) => Number.isNaN(n) || n < 0 || n > 255)) { | |
| return false; | |
| } | |
| const [a, b] = octets; | |
| // 10.0.0.0/8 | |
| if (a === 10) return true; | |
| // 172.16.0.0/12 | |
| if (a === 172 && b >= 16 && b <= 31) return true; | |
| // 192.168.0.0/16 | |
| if (a === 192 && b === 168) return true; | |
| // 127.0.0.0/8 (loopback) | |
| if (a === 127) return true; | |
| // 169.254.0.0/16 (link-local) | |
| if (a === 169 && b === 254) return true; | |
| // 0.0.0.0/8 | |
| if (a === 0) return true; | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/msteams/src/file-consent.ts
Line: 38-55
Comment:
IPv4 validation doesn't verify octets are 0-255, allowing invalid IPs like `999.999.999.999` to pass through
```suggestion
// IPv4 checks
const v4Parts = ip.split(".");
if (v4Parts.length === 4) {
const octets = v4Parts.map(Number);
// Validate all parts are valid 0-255
if (octets.some((n) => Number.isNaN(n) || n < 0 || n > 255)) {
return false;
}
const [a, b] = octets;
// 10.0.0.0/8
if (a === 10) return true;
// 172.16.0.0/12
if (a === 172 && b >= 16 && b <= 31) return true;
// 192.168.0.0/16
if (a === 192 && b === 168) return true;
// 127.0.0.0/8 (loopback)
if (a === 127) return true;
// 169.254.0.0/16 (link-local)
if (a === 169 && b === 254) return true;
// 0.0.0.0/8
if (a === 0) return true;
}
```
How can I resolve this? If you propose a fix, please make it concise.|
This pull request has been automatically marked as stale due to inactivity. |
Contributor
Author
|
The failing CI check (checks-windows) is unrelated to this PR — it's 3 tests in test/media-understanding.auto.test.ts (sherpa-onnx, whisper-cli, gemini CLI) that appear to be broken on the Windows runner independently. All 4804 other tests pass, including the 47 new file-consent tests. Can a maintainer re-run or skip the flaky check so this can move forward? |
Member
|
Hi @lewiswigmore — thanks for the submission. I’m the new Microsoft Teams maintainer for OpenClaw. Please give me a day or two to work through the open Teams backlog. Also, join the Twitter community for daily MS Teams feedback + updates: https://x.com/i/communities/2031170403607974228 |
1 similar comment
Member
|
Hi @lewiswigmore — thanks for the submission. I’m the new Microsoft Teams maintainer for OpenClaw. Please give me a day or two to work through the open Teams backlog. Also, join the Twitter community for daily MS Teams feedback + updates: https://x.com/i/communities/2031170403607974228 |
Contributor
Author
|
Hey @BradGroux - just checking in on this one. Let me know if there's anything else needed from my side. |
Member
|
Nice fix overall. I pushed a follow-up hardening commit.
This closes a potential bypass where one resolved answer is public but another is private or internal. Commit: |
The uploadToConsentUrl() function previously accepted any URL from the fileConsent/invoke response without validation. A malicious Teams tenant user could craft an invoke activity with an attacker-controlled uploadUrl, causing the bot to PUT file data to arbitrary destinations (SSRF). This commit adds validateConsentUploadUrl() which enforces: 1. HTTPS-only protocol 2. Hostname must match a strict allowlist of Microsoft/SharePoint domains (sharepoint.com, graph.microsoft.com, onedrive.com, etc.) 3. DNS resolution check rejects private/reserved IPs (RFC 1918, loopback, link-local) to prevent DNS rebinding attacks The CONSENT_UPLOAD_HOST_ALLOWLIST is intentionally narrower than the existing DEFAULT_MEDIA_HOST_ALLOWLIST, excluding overly broad domains like blob.core.windows.net and trafficmanager.net that any Azure customer can create endpoints under. Includes 47 tests covering IPv4/IPv6 private IP detection, protocol enforcement, hostname allowlist matching, DNS failure handling, and end-to-end upload validation.
5109047 to
90592be
Compare
steipete
pushed a commit
to leoleedev/openclaw
that referenced
this pull request
Apr 6, 2026
…w#23596) * fix(msteams): add SSRF validation to file consent upload URL The uploadToConsentUrl() function previously accepted any URL from the fileConsent/invoke response without validation. A malicious Teams tenant user could craft an invoke activity with an attacker-controlled uploadUrl, causing the bot to PUT file data to arbitrary destinations (SSRF). This commit adds validateConsentUploadUrl() which enforces: 1. HTTPS-only protocol 2. Hostname must match a strict allowlist of Microsoft/SharePoint domains (sharepoint.com, graph.microsoft.com, onedrive.com, etc.) 3. DNS resolution check rejects private/reserved IPs (RFC 1918, loopback, link-local) to prevent DNS rebinding attacks The CONSENT_UPLOAD_HOST_ALLOWLIST is intentionally narrower than the existing DEFAULT_MEDIA_HOST_ALLOWLIST, excluding overly broad domains like blob.core.windows.net and trafficmanager.net that any Azure customer can create endpoints under. Includes 47 tests covering IPv4/IPv6 private IP detection, protocol enforcement, hostname allowlist matching, DNS failure handling, and end-to-end upload validation. * fix(msteams): validate all DNS answers for consent uploads * fix(msteams): restore changelog header --------- Co-authored-by: Brad Groux <bradgroux@users.noreply.github.com>
Mlightsnow
pushed a commit
to Mlightsnow/openclaw
that referenced
this pull request
Apr 6, 2026
…w#23596) * fix(msteams): add SSRF validation to file consent upload URL The uploadToConsentUrl() function previously accepted any URL from the fileConsent/invoke response without validation. A malicious Teams tenant user could craft an invoke activity with an attacker-controlled uploadUrl, causing the bot to PUT file data to arbitrary destinations (SSRF). This commit adds validateConsentUploadUrl() which enforces: 1. HTTPS-only protocol 2. Hostname must match a strict allowlist of Microsoft/SharePoint domains (sharepoint.com, graph.microsoft.com, onedrive.com, etc.) 3. DNS resolution check rejects private/reserved IPs (RFC 1918, loopback, link-local) to prevent DNS rebinding attacks The CONSENT_UPLOAD_HOST_ALLOWLIST is intentionally narrower than the existing DEFAULT_MEDIA_HOST_ALLOWLIST, excluding overly broad domains like blob.core.windows.net and trafficmanager.net that any Azure customer can create endpoints under. Includes 47 tests covering IPv4/IPv6 private IP detection, protocol enforcement, hostname allowlist matching, DNS failure handling, and end-to-end upload validation. * fix(msteams): validate all DNS answers for consent uploads * fix(msteams): restore changelog header --------- Co-authored-by: Brad Groux <bradgroux@users.noreply.github.com>
lovewanwan
pushed a commit
to lovewanwan/openclaw
that referenced
this pull request
Apr 28, 2026
…w#23596) * fix(msteams): add SSRF validation to file consent upload URL The uploadToConsentUrl() function previously accepted any URL from the fileConsent/invoke response without validation. A malicious Teams tenant user could craft an invoke activity with an attacker-controlled uploadUrl, causing the bot to PUT file data to arbitrary destinations (SSRF). This commit adds validateConsentUploadUrl() which enforces: 1. HTTPS-only protocol 2. Hostname must match a strict allowlist of Microsoft/SharePoint domains (sharepoint.com, graph.microsoft.com, onedrive.com, etc.) 3. DNS resolution check rejects private/reserved IPs (RFC 1918, loopback, link-local) to prevent DNS rebinding attacks The CONSENT_UPLOAD_HOST_ALLOWLIST is intentionally narrower than the existing DEFAULT_MEDIA_HOST_ALLOWLIST, excluding overly broad domains like blob.core.windows.net and trafficmanager.net that any Azure customer can create endpoints under. Includes 47 tests covering IPv4/IPv6 private IP detection, protocol enforcement, hostname allowlist matching, DNS failure handling, and end-to-end upload validation. * fix(msteams): validate all DNS answers for consent uploads * fix(msteams): restore changelog header --------- Co-authored-by: Brad Groux <bradgroux@users.noreply.github.com>
ogt-redknie
pushed a commit
to ogt-redknie/OPENX
that referenced
this pull request
May 2, 2026
…w#23596) * fix(msteams): add SSRF validation to file consent upload URL The uploadToConsentUrl() function previously accepted any URL from the fileConsent/invoke response without validation. A malicious Teams tenant user could craft an invoke activity with an attacker-controlled uploadUrl, causing the bot to PUT file data to arbitrary destinations (SSRF). This commit adds validateConsentUploadUrl() which enforces: 1. HTTPS-only protocol 2. Hostname must match a strict allowlist of Microsoft/SharePoint domains (sharepoint.com, graph.microsoft.com, onedrive.com, etc.) 3. DNS resolution check rejects private/reserved IPs (RFC 1918, loopback, link-local) to prevent DNS rebinding attacks The CONSENT_UPLOAD_HOST_ALLOWLIST is intentionally narrower than the existing DEFAULT_MEDIA_HOST_ALLOWLIST, excluding overly broad domains like blob.core.windows.net and trafficmanager.net that any Azure customer can create endpoints under. Includes 47 tests covering IPv4/IPv6 private IP detection, protocol enforcement, hostname allowlist matching, DNS failure handling, and end-to-end upload validation. * fix(msteams): validate all DNS answers for consent uploads * fix(msteams): restore changelog header --------- Co-authored-by: Brad Groux <bradgroux@users.noreply.github.com>
github-actions Bot
pushed a commit
to Desicool/openclaw
that referenced
this pull request
May 9, 2026
…w#23596) * fix(msteams): add SSRF validation to file consent upload URL The uploadToConsentUrl() function previously accepted any URL from the fileConsent/invoke response without validation. A malicious Teams tenant user could craft an invoke activity with an attacker-controlled uploadUrl, causing the bot to PUT file data to arbitrary destinations (SSRF). This commit adds validateConsentUploadUrl() which enforces: 1. HTTPS-only protocol 2. Hostname must match a strict allowlist of Microsoft/SharePoint domains (sharepoint.com, graph.microsoft.com, onedrive.com, etc.) 3. DNS resolution check rejects private/reserved IPs (RFC 1918, loopback, link-local) to prevent DNS rebinding attacks The CONSENT_UPLOAD_HOST_ALLOWLIST is intentionally narrower than the existing DEFAULT_MEDIA_HOST_ALLOWLIST, excluding overly broad domains like blob.core.windows.net and trafficmanager.net that any Azure customer can create endpoints under. Includes 47 tests covering IPv4/IPv6 private IP detection, protocol enforcement, hostname allowlist matching, DNS failure handling, and end-to-end upload validation. * fix(msteams): validate all DNS answers for consent uploads * fix(msteams): restore changelog header --------- Co-authored-by: Brad Groux <bradgroux@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
uploadToConsentUrl()in the MS Teams plugin performs a PUT request to the URL provided in thefileConsent/invokeresponse without any validation. While the invoke activity is authenticated via JWT (Bot Framework webhook auth), a malicious user within an authenticated Teams tenant can craft an invoke with an attacker-controlleduploadUrl, causing the bot to PUT file data to arbitrary destinations — a server-side request forgery (SSRF).Attack scenario
fileConsent/invokeactivity withaction: "accept"anduploadInfo.uploadUrlpointing to an internal service (e.g.http://169.254.169.254/latest/meta-data/or an internal API)Fix
This PR adds
validateConsentUploadUrl()which enforces three checks before any fetch occurs:http://,file://, and other protocolsCONSENT_UPLOAD_HOST_ALLOWLISTrestricted to Microsoft/SharePoint domains that Teams legitimately provides as upload destinations (sharepoint.com,graph.microsoft.com,onedrive.com, etc.)::1,fe80::/10,fc00::/7) to prevent DNS rebinding attacksThe allowlist is intentionally narrower than
DEFAULT_MEDIA_HOST_ALLOWLIST, which includes overly broad domains likeblob.core.windows.netandtrafficmanager.netthat any Azure customer can create endpoints under.Testing
47 new tests covering:
uploadToConsentUrl()integration verifying fetch is never called for blocked URLsAll 201 existing msteams tests continue to pass with zero regressions.
Greptile Summary
Adds SSRF protection to MS Teams file consent upload flow by validating URLs before fetch with HTTPS enforcement, Microsoft/SharePoint domain allowlist, and DNS resolution checks to block private IPs. The fix addresses a real security vulnerability where authenticated Teams users could trigger bot PUT requests to arbitrary internal endpoints.
Key changes:
validateConsentUploadUrl()enforces protocol, hostname allowlist, and DNS-based IP checksCONSENT_UPLOAD_HOST_ALLOWLISTrestricts uploads to legitimate Microsoft domainsisPrivateOrReservedIP()blocks RFC 1918, loopback, and link-local rangesuploadToConsentUrl()ensures all uploads are validatedCritical issues found:
::ffff:10.0.0.1bypass private IP detectionBoth issues could allow SSRF attacks to succeed despite the validation layer.
Confidence Score: 2/5
extensions/msteams/src/file-consent.ts- the IP validation logic needs fixes before mergeLast reviewed commit: f260f9a
(2/5) Greptile learns from your feedback when you react with thumbs up/down!