🛡️ Sentinel: [HIGH] Fix SSRF vulnerability in web_fetch tool#585
🛡️ Sentinel: [HIGH] Fix SSRF vulnerability in web_fetch tool#585mudcube wants to merge 1 commit into
Conversation
🚨 Severity: HIGH 💡 Vulnerability: The `web_fetch` tool used a naive `reqwest::blocking::get` call which seamlessly follows redirects and resolves local/private hostnames, allowing internal network scanning and interaction (Server-Side Request Forgery). 🎯 Impact: An attacker could potentially access private network services, cloud metadata endpoints, or localhost services. 🔧 Fix: Implemented strict IP validation (`is_allowed_ip`) to block loopback, private networks, broadcast, and link-local IPs across both IPv4 and IPv6. Pinned the validated IP using `reqwest::blocking::ClientBuilder::resolve()` to prevent TOCTOU DNS rebinding, and implemented manual redirect tracking with the same validation loop. ✅ Verification: Tested locally using a scratch file with standard cargo test targeting `mill-plugin-system` verifying both IPv4/IPv6 blocking logic and redirect tracking behaviors. File compiling properly via cargo clippy and formatting applied via cargo fmt. Co-authored-by: mudcube <101564+mudcube@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Deploying typemill with
|
| Latest commit: |
7907ffc
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2ae33b32.typemill.pages.dev |
| Branch Preview URL: | https://sentinel-ssrf-web-fetch-5294.typemill.pages.dev |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7907ffc72a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if octets[0] >= 224 { | ||
| return false; | ||
| } | ||
| true |
There was a problem hiding this comment.
When web_fetch is given an IPv4 target in 100.64.0.0/10, this function falls through to true, so the new SSRF guard still permits direct requests to shared/private carrier-grade NAT space. Environments commonly use this range for internal VPC or service networks, and because this check is the only gate before the request is sent, an attacker can still reach those non-public services; the filter should reject shared/non-global IPv4 ranges rather than only RFC1918, loopback, link-local, and multicast.
Useful? React with 👍 / 👎.
1 participant
🛡️ Sentinel: [HIGH] Fix SSRF vulnerability in web_fetch tool
🚨 Severity: HIGH
💡 Vulnerability: The
web_fetchtool used a naivereqwest::blocking::getcall which seamlessly follows redirects and resolves local/private hostnames, allowing internal network scanning and interaction (Server-Side Request Forgery).🎯 Impact: An attacker could potentially access private network services, cloud metadata endpoints, or localhost services.
🔧 Fix: Implemented strict IP validation (
is_allowed_ip) to block loopback, private networks, broadcast, and link-local IPs across both IPv4 and IPv6. Pinned the validated IP usingreqwest::blocking::ClientBuilder::resolve()to prevent TOCTOU DNS rebinding, and implemented manual redirect tracking with the same validation loop.✅ Verification: Tested locally using a scratch file with standard cargo test targeting
mill-plugin-systemverifying both IPv4/IPv6 blocking logic and redirect tracking behaviors. File compiling properly via cargo clippy and formatting applied via cargo fmt.PR created automatically by Jules for task 5294791643917694713 started by @mudcube