From 4d830f707fc4314741fd431e70c2ce50cd5a3108 Mon Sep 17 00:00:00 2001 From: Henrique Dias Date: Thu, 26 Jun 2025 21:09:16 +0200 Subject: [PATCH] fix: correctly check if command is allowed when using shell --- http/commands.go | 14 ++++++++++++-- runner/parser.go | 33 ++++++++++++--------------------- runner/runner.go | 2 +- users/users.go | 10 ---------- 4 files changed, 25 insertions(+), 34 deletions(-) diff --git a/http/commands.go b/http/commands.go index 55075db4..1da1f75c 100644 --- a/http/commands.go +++ b/http/commands.go @@ -6,6 +6,7 @@ import ( "log" "net/http" "os/exec" + "slices" "strings" "time" @@ -60,7 +61,16 @@ var commandsHandler = withUser(func(w http.ResponseWriter, r *http.Request, d *d } } - command, err := runner.ParseCommand(d.settings, raw) + // Fail fast + if !d.server.EnableExec || !d.user.Perm.Execute { + if err := conn.WriteMessage(websocket.TextMessage, cmdNotAllowed); err != nil { //nolint:govet + wsErr(conn, r, http.StatusInternalServerError, err) + } + + return 0, nil + } + + command, name, err := runner.ParseCommand(d.settings, raw) if err != nil { if err := conn.WriteMessage(websocket.TextMessage, []byte(err.Error())); err != nil { //nolint:govet wsErr(conn, r, http.StatusInternalServerError, err) @@ -68,7 +78,7 @@ var commandsHandler = withUser(func(w http.ResponseWriter, r *http.Request, d *d return 0, nil } - if !d.server.EnableExec || !d.user.CanExecute(command[0]) { + if !slices.Contains(d.user.Commands, name) { if err := conn.WriteMessage(websocket.TextMessage, cmdNotAllowed); err != nil { //nolint:govet wsErr(conn, r, http.StatusInternalServerError, err) } diff --git a/runner/parser.go b/runner/parser.go index 8c7c13a2..54336847 100644 --- a/runner/parser.go +++ b/runner/parser.go @@ -1,33 +1,24 @@ package runner import ( - "os/exec" - "github.com/filebrowser/filebrowser/v2/settings" ) // ParseCommand parses the command taking in account if the current // instance uses a shell to run the commands or just calls the binary // directly. -func ParseCommand(s *settings.Settings, raw string) ([]string, error) { - var command []string - - if len(s.Shell) == 0 || s.Shell[0] == "" { - cmd, args, err := SplitCommandAndArgs(raw) - if err != nil { - return nil, err - } - - _, err = exec.LookPath(cmd) - if err != nil { - return nil, err - } - - command = append(command, cmd) - command = append(command, args...) - } else { - command = append(s.Shell, raw) //nolint:gocritic +func ParseCommand(s *settings.Settings, raw string) (command []string, name string, err error) { + name, args, err := SplitCommandAndArgs(raw) + if err != nil { + return } - return command, nil + if len(s.Shell) == 0 || s.Shell[0] == "" { + command = append(command, name) + command = append(command, args...) + } else { + command = append(s.Shell, raw) + } + + return command, name, nil } diff --git a/runner/runner.go b/runner/runner.go index 2dbafa5c..00e7c16a 100644 --- a/runner/runner.go +++ b/runner/runner.go @@ -60,7 +60,7 @@ func (r *Runner) exec(raw, evt, path, dst string, user *users.User) error { raw = strings.TrimSpace(strings.TrimSuffix(raw, "&")) } - command, err := ParseCommand(r.Settings, raw) + command, _, err := ParseCommand(r.Settings, raw) if err != nil { return err } diff --git a/users/users.go b/users/users.go index 01eb294c..e0310f21 100644 --- a/users/users.go +++ b/users/users.go @@ -2,7 +2,6 @@ package users import ( "path/filepath" - "slices" "github.com/spf13/afero" @@ -104,12 +103,3 @@ func (u *User) Clean(baseScope string, fields ...string) error { func (u *User) FullPath(path string) string { return afero.FullBaseFsPath(u.Fs.(*afero.BasePathFs), path) } - -// CanExecute checks if an user can execute a specific command. -func (u *User) CanExecute(command string) bool { - if !u.Perm.Execute { - return false - } - - return slices.Contains(u.Commands, command) -}