fix(security): add command whitelist to prevent command injection bypass

This commit addresses a critical security issue identified by CodeRabbit
where the sudo flag-skipping logic could be bypassed to execute arbitrary
commands (e.g., "sudo -u apt bash -c malicious_command").

Changes:
- Add allowedCommands whitelist for package managers
- Add allowedSudoCommands whitelist for commands after sudo/pkexec/doas
- Implement isCommandAllowed() with secure validation that rejects
  any sudo invocation with flags before the command
- Add comprehensive test cases including bypass attack scenarios

The fix follows CodeRabbit's recommendation to not attempt parsing
sudo flags, instead requiring the package manager to immediately
follow the privilege escalation command.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Lea Anthony 2026-01-25 11:20:59 +11:00
commit b97ca22a48
2 changed files with 142 additions and 6 deletions

View file

@ -544,15 +544,90 @@ type InstallResponse struct {
Error string `json:"error,omitempty"`
}
// allowedCommands is a whitelist of commands that can be executed for dependency installation.
// This prevents arbitrary command execution even though commands originate from backend detection.
var allowedCommands = map[string]bool{
// Package managers (may be called directly or via sudo)
"apt": true,
"apt-get": true,
"apk": true, // Alpine Linux
"dnf": true,
"yum": true,
"pacman": true,
"zypper": true,
"emerge": true,
"eopkg": true,
"nix-env": true,
"brew": true,
"port": true, // MacPorts
"winget": true,
"choco": true,
"scoop": true,
"snap": true,
"flatpak": true,
"xcode-select": true, // macOS Xcode CLI tools
// Privilege escalation (validated separately)
"sudo": true,
"pkexec": true,
"doas": true,
}
// allowedSudoCommands are commands allowed to be run after sudo/pkexec/doas.
// This is a subset of allowedCommands - privilege escalation wrappers are not allowed here.
var allowedSudoCommands = map[string]bool{
"apt": true,
"apt-get": true,
"apk": true,
"dnf": true,
"yum": true,
"pacman": true,
"zypper": true,
"emerge": true,
"eopkg": true,
"nix-env": true,
"brew": true,
"port": true,
"snap": true,
"flatpak": true,
"xcode-select": true,
}
// isCommandAllowed checks if a command is in the whitelist.
// For sudo/pkexec/doas, it validates the actual command being elevated.
// To avoid bypass attacks via flag parsing (e.g., "sudo -u apt bash"),
// we reject any sudo invocation where the second argument starts with "-".
func isCommandAllowed(parts []string) bool {
if len(parts) == 0 {
return false
}
cmd := parts[0]
if !allowedCommands[cmd] {
return false
}
// If it's a privilege escalation command, validate the actual command
if cmd == "sudo" || cmd == "pkexec" || cmd == "doas" {
if len(parts) < 2 {
return false
}
// Reject any flags before the command to prevent bypass attacks
// like "sudo -u apt bash" where -u takes "apt" as its argument
actualCmd := parts[1]
if strings.HasPrefix(actualCmd, "-") {
return false
}
return allowedSudoCommands[actualCmd]
}
return true
}
// handleInstallDependency executes dependency installation commands.
//
// Security note: This endpoint executes commands that originate from the backend's
// package manager detection (see packagemanager.InstallCommand). The commands are
// generated server-side based on the detected OS package manager, not from arbitrary
// user input. This is a local development tool where the explicit purpose is to help
// users install dependencies on their own machine. The frontend currently only uses
// this data to copy commands to clipboard - this endpoint exists for potential future
// use or automation scenarios.
// validated against a whitelist of allowed package managers before execution.
func (w *Wizard) handleInstallDependency(rw http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodPost {
http.Error(rw, "Method not allowed", http.StatusMethodNotAllowed)
@ -578,7 +653,16 @@ func (w *Wizard) handleInstallDependency(rw http.ResponseWriter, r *http.Request
return
}
cmd := exec.Command(parts[0], parts[1:]...) // #nosec G204 -- commands originate from backend package manager detection, not user input
// Validate command against whitelist before execution
if !isCommandAllowed(parts) {
json.NewEncoder(rw).Encode(InstallResponse{
Success: false,
Error: "Command not allowed: only package manager commands are permitted",
})
return
}
cmd := exec.Command(parts[0], parts[1:]...) // #nosec G204 -- validated against allowedCommands whitelist
output, err := cmd.CombinedOutput()
if err != nil {

View file

@ -0,0 +1,52 @@
package setupwizard
import "testing"
func TestIsCommandAllowed(t *testing.T) {
tests := []struct {
name string
parts []string
allowed bool
}{
// Valid package manager commands
{"apt install", []string{"apt", "install", "pkg"}, true},
{"apt-get install", []string{"apt-get", "install", "pkg"}, true},
{"brew install", []string{"brew", "install", "pkg"}, true},
{"pacman -S", []string{"pacman", "-S", "pkg"}, true},
{"dnf install", []string{"dnf", "install", "pkg"}, true},
// Valid sudo commands
{"sudo apt install", []string{"sudo", "apt", "install", "pkg"}, true},
{"sudo apt-get install", []string{"sudo", "apt-get", "install", "pkg"}, true},
{"sudo pacman -S", []string{"sudo", "pacman", "-S", "pkg"}, true},
{"pkexec apt install", []string{"pkexec", "apt", "install", "pkg"}, true},
{"doas apt install", []string{"doas", "apt", "install", "pkg"}, true},
// CRITICAL: Bypass attempts that MUST be blocked
{"sudo -u apt bash (bypass)", []string{"sudo", "-u", "apt", "bash", "-c", "malicious"}, false},
{"sudo -E bash", []string{"sudo", "-E", "bash"}, false},
{"sudo --user=root bash", []string{"sudo", "--user=root", "bash"}, false},
{"doas -u apt bash", []string{"doas", "-u", "apt", "bash"}, false},
{"pkexec --user apt bash", []string{"pkexec", "--user", "apt", "bash"}, false},
// Invalid commands
{"bash", []string{"bash", "-c", "malicious"}, false},
{"rm -rf", []string{"rm", "-rf", "/"}, false},
{"curl", []string{"curl", "http://evil.com"}, false},
{"wget", []string{"wget", "http://evil.com"}, false},
{"empty", []string{}, false},
{"sudo only", []string{"sudo"}, false},
// Nested sudo attempts
{"sudo sudo apt", []string{"sudo", "sudo", "apt"}, false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := isCommandAllowed(tt.parts)
if got != tt.allowed {
t.Errorf("isCommandAllowed(%v) = %v, want %v", tt.parts, got, tt.allowed)
}
})
}
}