fix(auth): reject signup when normalized home dir collides (GHSA-7rc3-g7h6-22m7)

cleanUsername is many-to-one, so distinct usernames (e.g. "teamone/x" and
"teamone-x") can normalize to the same home directory. With CreateUserDir
enabled, the second registrant silently reused the first user's directory,
breaking per-user isolation. Add a GetByScope lookup and reject a signup whose
derived scope is already taken. The check is gated on CreateUserDir: when it is
off, signups intentionally share the configured default scope. Adds a regression
test for the colliding-username case.

auth/proxy_test.go

@@ -22,6 +22,7 @@ func (m *mockUserStore) Get(_ string, _ bool, id interface{}) (*users.User, erro
 	return nil, fberrors.ErrNotExist
 }
 
+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 {

http/auth.go

@@ -201,6 +201,22 @@ var signupHandler = func(w http.ResponseWriter, r *http.Request, d *data) (int,
 		return http.StatusInternalServerError, err
 	}
 	user.Scope = userHome
+
+	// When home directories are created from the username, distinct usernames
+	// can normalize to the same scope (cleanUsername is many-to-one), which would
+	// silently hand the new user another user's home directory. Reject the signup
+	// if the derived scope is already taken. When CreateUserDir is off, all
+	// signups intentionally share the configured default scope, so this check
+	// does not apply.
+	if d.settings.CreateUserDir {
+		switch _, err := d.store.Users.GetByScope(user.Scope); {
+		case err == nil:
+			return http.StatusConflict, fberrors.ErrExist
+		case !errors.Is(err, fberrors.ErrNotExist):
+			return http.StatusInternalServerError, err
+		}
+	}
+
 	log.Printf("new user: %s, home dir: [%s].", user.Username, userHome)
 
 	err = d.store.Users.Save(user)

http/auth_test.go

@@ -0,0 +1,71 @@
+package fbhttp
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/asdine/storm/v3"
+
+	"github.com/filebrowser/filebrowser/v2/settings"
+	"github.com/filebrowser/filebrowser/v2/storage/bolt"
+)
+
+// Regression for the username-normalization home-directory collision
+// (GHSA-7rc3-g7h6-22m7): with Signup and CreateUserDir enabled, two distinct
+// usernames that cleanUsername() normalizes to the same directory must not be
+// handed the same home directory. The second registration is rejected.
+func TestSignupRejectsCollidingNormalizedScope(t *testing.T) {
+	root := t.TempDir()
+
+	db, err := storm.Open(filepath.Join(t.TempDir(), "db"))
+	if err != nil {
+		t.Fatalf("failed to open db: %v", err)
+	}
+	t.Cleanup(func() { _ = db.Close() })
+
+	st, err := bolt.NewStorage(db)
+	if err != nil {
+		t.Fatalf("failed to get storage: %v", err)
+	}
+	if err := st.Settings.Save(&settings.Settings{
+		Key:                   []byte("test-signing-key"),
+		Signup:                true,
+		CreateUserDir:         true,
+		UserHomeBasePath:      "/users",
+		MinimumPasswordLength: 1,
+	}); err != nil {
+		t.Fatalf("failed to save settings: %v", err)
+	}
+
+	server := &settings.Server{Root: root}
+
+	signup := func(username string) *httptest.ResponseRecorder {
+		body := `{"username":"` + username + `","password":"CollidePw12345!"}`
+		req, _ := http.NewRequest(http.MethodPost, "/signup", strings.NewReader(body))
+		rec := httptest.NewRecorder()
+		handle(signupHandler, "", st, server).ServeHTTP(rec, req)
+		return rec
+	}
+
+	// Victim registers first and gets /users/teamone-x.
+	if rec := signup("teamone-x"); rec.Code != http.StatusOK {
+		t.Fatalf("first signup: expected 200, got %d body=%q", rec.Code, rec.Body.String())
+	}
+
+	// Attacker picks a distinct username that normalizes to the same scope.
+	if rec := signup("teamone/x"); rec.Code != http.StatusConflict {
+		t.Fatalf("VULNERABLE: colliding signup expected 409, got %d body=%q", rec.Code, rec.Body.String())
+	}
+
+	// The shared scope must still be owned solely by the first user.
+	owner, err := st.Users.GetByScope("/users/teamone-x")
+	if err != nil {
+		t.Fatalf("expected first user to own the scope: %v", err)
+	}
+	if owner.Username != "teamone-x" {
+		t.Fatalf("scope owner = %q, want teamone-x", owner.Username)
+	}
+}

storage/bolt/users.go

@@ -6,6 +6,7 @@ import (
 	"reflect"
 
 	"github.com/asdine/storm/v3"
+	"github.com/asdine/storm/v3/q"
 	bolt "go.etcd.io/bbolt"
 
 	fberrors "github.com/filebrowser/filebrowser/v2/errors"
@@ -41,6 +42,19 @@ func (st usersBackend) GetBy(i interface{}) (user *users.User, err error) {
 	return
 }
 
+func (st usersBackend) GetByScope(scope string) (*users.User, error) {
+	user := &users.User{}
+	err := st.db.Select(q.Eq("Scope", scope)).First(user)
+	if err != nil {
+		if errors.Is(err, storm.ErrNotFound) {
+			return nil, fberrors.ErrNotExist
+		}
+		return nil, err
+	}
+
+	return user, nil
+}
+
 func (st usersBackend) Gets() ([]*users.User, error) {
 	var allUsers []*users.User
 	err := st.db.All(&allUsers)

users/storage.go

@@ -10,6 +10,7 @@ import (
 // StorageBackend is the interface to implement for a users storage.
 type StorageBackend interface {
 	GetBy(interface{}) (*User, error)
+	GetByScope(scope string) (*User, error)
 	Gets() ([]*User, error)
 	Save(u *User) error
 	Update(u *User, fields ...string) error
@@ -20,6 +21,7 @@ type StorageBackend interface {
 
 type Store interface {
 	Get(baseScope string, followExternalSymlinks bool, id interface{}) (user *User, err error)
+	GetByScope(scope string) (*User, error)
 	Gets(baseScope string, followExternalSymlinks bool) ([]*User, error)
 	Update(user *User, fields ...string) error
 	Save(user *User) error
@@ -56,6 +58,13 @@ func (s *Storage) Get(baseScope string, followExternalSymlinks bool, id interfac
 	return
 }
 
+// GetByScope returns the first user whose scope matches the given one, or
+// ErrNotExist if none does. The user is returned as stored, without setting up
+// its filesystem, as it is meant for existence checks rather than serving.
+func (s *Storage) GetByScope(scope string) (*User, error) {
+	return s.back.GetByScope(scope)
+}
+
 // Gets gets a list of all users.
 func (s *Storage) Gets(baseScope string, followExternalSymlinks bool) ([]*User, error) {
 	users, err := s.back.Gets()