Security, scope, and share access fixes

Harden symlink, archive, and auth checks
Fix copy/move, uploads, and file handling
Improve path, listing, and share behavior
UI cleanup and parser reliability updates

Author: dormant-user

auth/hook.go

@@ -68,7 +68,7 @@ func (a *HookAuth) Auth(r *http.Request, usr users.Store, stg *settings.Settings
 	case "block":
 		return nil, os.ErrPermission
 	case "pass":
-		u, err := a.Users.Get(a.Server.Root, a.Cred.Username)
+		u, err := a.Users.Get(a.Server.Root, a.Server.FollowExternalSymlinks, a.Cred.Username)
 		if err != nil || !users.CheckPwd(a.Cred.Password, u.Password) {
 			return nil, os.ErrPermission
 		}
@@ -86,22 +86,6 @@ func (a *HookAuth) LoginPage() bool {
 // RunCommand starts the hook command and returns the action
 func (a *HookAuth) RunCommand() (string, error) {
 	command := strings.Split(a.Command, " ")
-	envMapping := func(key string) string {
-		switch key {
-		case "USERNAME":
-			return a.Cred.Username
-		case "PASSWORD":
-			return a.Cred.Password
-		default:
-			return os.Getenv(key)
-		}
-	}
-	for i, arg := range command {
-		if i == 0 {
-			continue
-		}
-		command[i] = os.Expand(arg, envMapping)
-	}
 
 	cmd := exec.Command(command[0], command[1:]...)
 	cmd.Env = append(os.Environ(), fmt.Sprintf("USERNAME=%s", a.Cred.Username))
@@ -145,7 +129,7 @@ func (a *HookAuth) GetValues(s string) {
 
 // SaveUser updates the existing user or creates a new one when not found
 func (a *HookAuth) SaveUser() (*users.User, error) {
-	u, err := a.Users.Get(a.Server.Root, a.Cred.Username)
+	u, err := a.Users.Get(a.Server.Root, a.Server.FollowExternalSymlinks, a.Cred.Username)
 	if err != nil && !errors.Is(err, fberrors.ErrNotExist) {
 		return nil, err
 	}

auth/hook_test.go

@@ -0,0 +1,88 @@
+package auth
+
+import (
+	"os"
+	"path/filepath"
+	"runtime"
+	"testing"
+)
+
+// writeHookScript writes a POSIX shell script to a temp file and returns its
+// path, marking it executable.
+func writeHookScript(t *testing.T, body string) string {
+	t.Helper()
+	path := filepath.Join(t.TempDir(), "hook.sh")
+	if err := os.WriteFile(path, []byte("#!/bin/sh\n"+body), 0o700); err != nil {
+		t.Fatalf("failed to write hook script: %v", err)
+	}
+	return path
+}
+
+// TestRunCommandNoCredentialInjection ensures that attacker-controlled
+// credentials submitted at the unauthenticated login endpoint cannot be
+// injected into the hook command string. Credentials must only ever reach the
+// hook through the USERNAME/PASSWORD environment variables, never via string
+// substitution into the command itself (CWE-78/CWE-88).
+func TestRunCommandNoCredentialInjection(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("uses POSIX shell")
+	}
+
+	marker := filepath.Join(t.TempDir(), "pwned")
+
+	// The hook simply blocks. If the credential were ever interpolated into the
+	// command string and evaluated by a shell, the embedded `touch` would
+	// create the marker file.
+	script := writeHookScript(t, "echo hook.action=block\n")
+
+	a := &HookAuth{
+		Command: script,
+		Cred: hookCred{
+			Username: `"; touch ` + marker + `; #`,
+			Password: `$(touch ` + marker + `)`,
+		},
+	}
+
+	action, err := a.RunCommand()
+	if err != nil {
+		t.Fatalf("RunCommand returned error: %v", err)
+	}
+	if action != "block" {
+		t.Fatalf("expected action %q, got %q", "block", action)
+	}
+	if _, err := os.Stat(marker); err == nil {
+		t.Fatalf("credential injection executed: marker file %q was created", marker)
+	}
+}
+
+// TestRunCommandReceivesCredentialsViaEnv verifies the supported contract: the
+// hook receives credentials through the USERNAME and PASSWORD environment
+// variables.
+func TestRunCommandReceivesCredentialsViaEnv(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("uses POSIX shell")
+	}
+
+	script := writeHookScript(t, `if [ "$USERNAME" = alice ] && [ "$PASSWORD" = secret ]; then
+  echo hook.action=auth
+else
+  echo hook.action=block
+fi
+`)
+
+	a := &HookAuth{
+		Command: script,
+		Cred: hookCred{
+			Username: "alice",
+			Password: "secret",
+		},
+	}
+
+	action, err := a.RunCommand()
+	if err != nil {
+		t.Fatalf("RunCommand returned error: %v", err)
+	}
+	if action != "auth" {
+		t.Fatalf("expected action %q, got %q", "auth", action)
+	}
+}

auth/json.go

@@ -206,7 +206,7 @@ func (a JSONAuth) Auth(r *http.Request, usr users.Store, _ *settings.Settings, s
 	cred, err := extractCredentials(authHeader)
 	if err != nil {
 		log.Printf("Warning: Failed to extract credentials. %s", err)
-		return nil, err
+		return nil, os.ErrPermission
 	}
 
 	// If ReCaptcha is enabled, check the code.
@@ -222,7 +222,7 @@ func (a JSONAuth) Auth(r *http.Request, usr users.Store, _ *settings.Settings, s
 		}
 	}
 
-	u, err := usr.Get(srv.Root, cred.Username)
+	u, err := usr.Get(srv.Root, srv.FollowExternalSymlinks, cred.Username)
 	if err != nil {
 		log.Printf("Warning: Login error for %s - lookup failed: %v", cred.Username, err)
 		// Even if the user is not found, we check the password against a dummy hash

auth/none.go

@@ -15,7 +15,7 @@ type NoAuth struct{}
 
 // Auth uses authenticates user 1.
 func (a NoAuth) Auth(_ *http.Request, usr users.Store, _ *settings.Settings, srv *settings.Server) (*users.User, error) {
-	return usr.Get(srv.Root, uint(1))
+	return usr.Get(srv.Root, srv.FollowExternalSymlinks, uint(1))
 }
 
 // LoginPage tells that no auth doesn't require a login page.

auth/proxy.go

@@ -20,7 +20,7 @@ type ProxyAuth struct {
 // Auth authenticates the user via an HTTP header.
 func (a ProxyAuth) Auth(r *http.Request, usr users.Store, setting *settings.Settings, srv *settings.Server) (*users.User, error) {
 	username := r.Header.Get(a.Header)
-	user, err := usr.Get(srv.Root, username)
+	user, err := usr.Get(srv.Root, srv.FollowExternalSymlinks, username)
 	if errors.Is(err, fberrors.ErrNotExist) {
 		return a.createUser(usr, setting, srv, username)
 	}

auth/proxy_test.go

@@ -13,7 +13,7 @@ type mockUserStore struct {
 	users map[string]*users.User
 }
 
-func (m *mockUserStore) Get(_ string, id interface{}) (*users.User, error) {
+func (m *mockUserStore) Get(_ string, _ bool, id interface{}) (*users.User, error) {
 	if v, ok := id.(string); ok {
 		if u, ok := m.users[v]; ok {
 			return u, nil
@@ -22,14 +22,15 @@ func (m *mockUserStore) Get(_ string, id interface{}) (*users.User, error) {
 	return nil, fberrors.ErrNotExist
 }
 
-func (m *mockUserStore) Gets(_ string) ([]*users.User, error) { return nil, nil }
-func (m *mockUserStore) Update(_ *users.User, _ ...string) error { return nil }
+func (m *mockUserStore) GetByScope(_ string) (*users.User, error)     { return nil, fberrors.ErrNotExist }
+func (m *mockUserStore) Gets(_ string, _ bool) ([]*users.User, error) { return nil, nil }
+func (m *mockUserStore) Update(_ *users.User, _ ...string) error      { return nil }
 func (m *mockUserStore) Save(user *users.User) error {
 	m.users[user.Username] = user
 	return nil
 }
 func (m *mockUserStore) Delete(_ interface{}) error { return nil }
-func (m *mockUserStore) LastUpdate(_ uint) int64     { return 0 }
+func (m *mockUserStore) LastUpdate(_ uint) int64    { return 0 }
 
 func TestProxyAuthCreateUserRestrictsDefaults(t *testing.T) {
 	t.Parallel()

cmd/config.go

@@ -244,6 +244,7 @@ func printSettings(ser *settings.Server, set *settings.Settings, auther auth.Aut
 	fmt.Fprintf(w, "\tThumbnails Enabled:\t%t\n", ser.EnableThumbnails)
 	fmt.Fprintf(w, "\tResize Preview:\t%t\n", ser.ResizePreview)
 	fmt.Fprintf(w, "\tType Detection by Header:\t%t\n", ser.TypeDetectionByHeader)
+	fmt.Fprintf(w, "\tFollow External Symlinks:\t%t\n", ser.FollowExternalSymlinks)
 
 	fmt.Fprintln(w, "\nTUS:")
 	fmt.Fprintf(w, "\tChunk size:\t%d\n", set.Tus.ChunkSize)

cmd/root.go

@@ -13,6 +13,7 @@ import (
 	"os"
 	"os/signal"
 	"path/filepath"
+	"strings"
 	"syscall"
 	"time"
 
@@ -111,6 +112,7 @@ func addServerFlags(flags *pflag.FlagSet) {
 	flags.Bool("disableExec", true, "disables Command Runner feature")
 	flags.Bool("disableTypeDetectionByHeader", false, "disables type detection by reading file headers")
 	flags.Bool("disableImageResolutionCalc", false, "disables image resolution calculation by reading image files")
+	flags.Bool("followExternalSymlinks", false, "follow symlinks whose target is outside the user scope (unsafe)")
 }
 
 var rootCmd = &cobra.Command{
@@ -362,6 +364,10 @@ func getServerSettings(v *viper.Viper, st *storage.Storage) (*settings.Server, e
 		server.EnableExec = !v.GetBool("disableExec")
 	}
 
+	if v.IsSet("followExternalSymlinks") {
+		server.FollowExternalSymlinks = v.GetBool("followExternalSymlinks")
+	}
+
 	if isAddrSet && isSocketSet {
 		return nil, errors.New("--socket flag cannot be used with --address, --port, --key nor --cert")
 	}
@@ -378,6 +384,25 @@ func getServerSettings(v *viper.Viper, st *storage.Storage) (*settings.Server, e
 		log.Println("WARNING: read https://github.com/thevickypedia/filebrowser/issues/5199")
 	}
 
+	if server.FollowExternalSymlinks {
+		log.Println("WARNING: Following external symlinks enabled!")
+		log.Println("WARNING: Symlinks pointing outside a user's scope will be followed,")
+		log.Println("WARNING: which can expose files outside that scope. Only enable this if")
+		log.Println("WARNING: you fully understand and trust the contents of every user scope.")
+	}
+
+	if set, err := st.Settings.Get(); err == nil && set.Signup {
+		scope := strings.TrimSpace(set.Defaults.Scope)
+		scopeIsRoot := scope == "" || scope == "." || scope == "/"
+
+		if !set.CreateUserDir && scopeIsRoot {
+			log.Println("WARNING: Signup is enabled without createUserDir and the default scope is")
+			log.Println("WARNING: the server root, so every self-registered user can read, modify and")
+			log.Println("WARNING: delete all files File Browser serves, including other users' files.")
+			log.Println("WARNING: Enable createUserDir, or set a default scope other than the root.")
+		}
+	}
+
 	return server, nil
 }
 
@@ -455,19 +480,20 @@ func quickSetup(v *viper.Viper, s *storage.Storage) error {
 	}
 
 	ser := &settings.Server{
-		BaseURL:               v.GetString("baseURL"),
-		Port:                  v.GetString("port"),
-		Log:                   v.GetString("log"),
-		TLSKey:                v.GetString("key"),
-		TLSCert:               v.GetString("cert"),
-		Address:               v.GetString("address"),
-		Root:                  v.GetString("root"),
-		TokenExpirationTime:   v.GetString("tokenExpirationTime"),
-		EnableThumbnails:      !v.GetBool("disableThumbnails"),
-		ResizePreview:         !v.GetBool("disablePreviewResize"),
-		EnableExec:            !v.GetBool("disableExec"),
-		TypeDetectionByHeader: !v.GetBool("disableTypeDetectionByHeader"),
-		ImageResolutionCal:    !v.GetBool("disableImageResolutionCalc"),
+		BaseURL:                v.GetString("baseURL"),
+		Port:                   v.GetString("port"),
+		Log:                    v.GetString("log"),
+		TLSKey:                 v.GetString("key"),
+		TLSCert:                v.GetString("cert"),
+		Address:                v.GetString("address"),
+		Root:                   v.GetString("root"),
+		TokenExpirationTime:    v.GetString("tokenExpirationTime"),
+		EnableThumbnails:       !v.GetBool("disableThumbnails"),
+		ResizePreview:          !v.GetBool("disablePreviewResize"),
+		EnableExec:             !v.GetBool("disableExec"),
+		TypeDetectionByHeader:  !v.GetBool("disableTypeDetectionByHeader"),
+		ImageResolutionCal:     !v.GetBool("disableImageResolutionCalc"),
+		FollowExternalSymlinks: v.GetBool("followExternalSymlinks"),
 	}
 
 	err = s.Settings.SaveServer(ser)

cmd/rules.go

@@ -36,7 +36,7 @@ func runRules(st *storage.Storage, cmd *cobra.Command, usersFn func(*users.User)
 	}
 	if id != nil {
 		var user *users.User
-		user, err = st.Users.Get("", id)
+		user, err = st.Users.Get("", false, id)
 		if err != nil {
 			return err
 		}

cmd/users.go

@@ -80,7 +80,7 @@ func addUserFlags(flags *pflag.FlagSet) {
 	flags.Bool("singleClick", false, "use single clicks only")
 	flags.Bool("redirectAfterCopyMove", false, "redirect to destination after copy/move")
 	flags.Bool("dateFormat", false, "use date format (true for absolute time, false for relative)")
-	flags.Bool("hideDotfiles", false, "hide dotfiles")
+	flags.Bool("hideDotfiles", false, "hide dotfiles in file listings")
 	flags.String("aceEditorTheme", "", "ace editor's syntax highlighting theme for users")
 }