-
Notifications
You must be signed in to change notification settings - Fork 4
Search for xdg-open in locations outside of PATH #84
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Kusari Analysis Results:Caution Flagged Issues Detected While dependency analysis shows no security concerns with pinned versions, code analysis identified a HIGH impact command injection vulnerability in pkg/safebrowse/safebrowse.go at line 224. The rawURL parameter passed to exec.CommandContext lacks proper validation and sanitization, potentially allowing malicious URLs containing shell metacharacters to execute arbitrary commands. This critical security risk must be addressed before merge by implementing URL validation and sanitization as outlined in the provided mitigation. The detected secret is confirmed as a false positive test data. Note View full detailed analysis result for more information on the output and the checks that were run. Required Code MitigationsAdd URL validation and sanitization before executing the command. Parse and validate the URL to ensure it's properly formatted and doesn't contain shell metacharacters that could be exploited for command injection.
Found this helpful? Give it a 👍 or 👎 reaction! |
cmd/goose/ui.go
Outdated
| return errors.New("xdg-open not found") | ||
| } | ||
| slog.Debug("Executing command", "command", xdgOpenPath, "url", rawURL) | ||
| cmd = exec.CommandContext(ctx, xdgOpenPath, rawURL) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Issue: Add URL validation and sanitization before executing the command. Parse and validate the URL to ensure it's properly formatted and doesn't contain shell metacharacters that could be exploited for command injection.
Recommended Code Changes:
// Validate and parse the URL before execution
parsedURL, err := url.Parse(rawURL)
if err != nil {
return fmt.Errorf("invalid URL: %w", err)
}
// Ensure the URL scheme is safe (http/https)
if parsedURL.Scheme != "http" && parsedURL.Scheme != "https" {
return errors.New("unsupported URL scheme")
}
// Use the cleaned URL string
cleanURL := parsedURL.String()
cmd = exec.CommandContext(ctx, xdgOpenPath, cleanURL)
|
Kusari PR Analysis rerun based on - 6dd7fae performed at: 2025-11-13T02:59:28Z - link to updated analysis |
| if err != nil { | ||
| return err | ||
| } | ||
| cmd = exec.CommandContext(ctx, xdgOpen, rawURL) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Issue: Add URL validation and sanitization before executing the command. Parse and validate the URL to ensure it's properly formatted and doesn't contain shell metacharacters that could be exploited for command injection.
Recommended Code Changes:
// Validate and parse the URL before execution
parsedURL, err := url.Parse(rawURL)
if err != nil {
return fmt.Errorf("invalid URL: %w", err)
}
// Ensure the URL scheme is safe (http/https)
if parsedURL.Scheme != "http" && parsedURL.Scheme != "https" {
return errors.New("unsupported URL scheme")
}
// Use the cleaned URL string
cleanURL := parsedURL.String()
cmd = exec.CommandContext(ctx, xdgOpen, cleanURL)
No description provided.