🛡️ Sentinel: [CRITICAL] Fix SSRF and Command Injection in browser URL handling#223
🛡️ Sentinel: [CRITICAL] Fix SSRF and Command Injection in browser URL handling#223
Conversation
… handling Fixed a vulnerability where `exec.Command` in `browser.go` was susceptible to Command Injection and SSRF via OS URL handlers. Handlers opened untrusted URLs directly with `open` and `rundll32`, allowing an attacker to evaluate malicious paths or schemes like `file:///` or `javascript:`. The fix uses `net/url.Parse()` to safely validate that the URL scheme is strictly `http` or `https` before attempting to execute platform-specific commands. The `url` parameter was renamed to `rawurl` to avoid shadowing the `net/url` standard library. A comprehensive suite of unit tests was also added to ensure invalid schemes are appropriately rejected. Co-authored-by: rschumann <360788+rschumann@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. |
1 participant
🚨 Severity: CRITICAL
💡 Vulnerability: SSRF and Command Injection in OS URL Handlers (
internal/browser/browser.go)🎯 Impact: An attacker could provide a malicious URL scheme (e.g.,
file:///etc/passwd,javascript:, orsmb://) to theOpenURLfunction, which gets passed directly to underlying platform execution commands (open,rundll32). This allows for arbitrary command execution or local file access.🔧 Fix: Renamed the parameter
urltorawurlto avoid shadowing the standardnet/urlpackage. Added validation usingnet/url.Parse(rawurl)to strictly whitelist thehttpandhttpsschemes, rejecting any URL attempting to use dangerous fallback protocols before execution.✅ Verification: Unit tests have been added (
internal/browser/browser_test.go) explicitly testing rejection of malicious schemes likefile://,ftp://, andjavascript:. Rungo test ./internal/browser/...to verify the fix.PR created automatically by Jules for task 7128643166862062418 started by @rschumann