fix: incomplete fix for symlinked directories let scopes users and public-share recipients read and write files outside of scope

files/file.go

@@ -128,22 +128,18 @@ func stat(opts *FileOptions) (*FileInfo, error) {
 		}
 	}
 
-	// regular file
-	if file != nil && !file.IsSymlink {
-		return file, nil
-	}
-
-	// The path is a symlink. Refuse to follow it if its on-disk target escapes
-	// the user's scoped root; otherwise a symlink that lives lexically inside
-	// the scope but points outside it would let a restricted user read, write,
-	// or share files beyond their boundary.
-	if file != nil && file.IsSymlink {
+	if file != nil {
 		ok, scopeErr := WithinScope(opts.Fs, opts.Path)
 		if scopeErr != nil || !ok {
 			return nil, os.ErrPermission
 		}
 	}
 
+	// regular file
+	if file != nil && !file.IsSymlink {
+		return file, nil
+	}
+
 	// fs doesn't support afero.Lstater interface or the file is a symlink
 	info, err := opts.Fs.Stat(opts.Path)
 	if err != nil {
@@ -479,6 +475,13 @@ func (i *FileInfo) readListing(checker rules.Checker, readHeader bool, calcImgRe
 		isSymlink, isInvalidLink := false, false
 		if IsSymlink(f.Mode()) {
 			isSymlink = true
+			// A symlink whose on-disk target escapes the scoped root must not be
+			// followed, otherwise the listing would leak the target's metadata
+			// (and downstream access) for files outside the user's scope or the
+			// shared subtree.
+			if ok, scopeErr := WithinScope(i.Fs, fPath); scopeErr != nil || !ok {
+				continue
+			}
 			// It's a symbolic link. We try to follow it. If it doesn't work,
 			// we stay with the link information instead of the target's.
 			info, err := i.Fs.Stat(fPath)

files/file_test.go

@@ -80,4 +80,54 @@ func TestWithinScope(t *testing.T) {
 			t.Fatal("expected escaping symlink to a sibling directory to be rejected")
 		}
 	})
+
+	t.Run("symlink whose target stays within scope is allowed", func(t *testing.T) {
+		scope := t.TempDir()
+		if err := os.MkdirAll(filepath.Join(scope, "real"), 0o755); err != nil {
+			t.Fatal(err)
+		}
+		if err := os.WriteFile(filepath.Join(scope, "real", "f.txt"), []byte("x"), 0o644); err != nil {
+			t.Fatal(err)
+		}
+		if err := os.Symlink(filepath.Join(scope, "real"), filepath.Join(scope, "link")); err != nil {
+			t.Skipf("cannot create symlink: %v", err)
+		}
+		bfs := afero.NewBasePathFs(afero.NewOsFs(), scope)
+
+		ok, err := WithinScope(bfs, "/link/f.txt")
+		if err != nil || !ok {
+			t.Fatalf("expected (true, nil) for an in-scope symlink target, got (%v, %v)", ok, err)
+		}
+	})
+}
+
+// stat must reject a regular file reached through a symlinked ancestor that
+// escapes the scope (GHSA-hf77-9m7w-fq8q), while still serving in-scope files.
+func TestStatRejectsLinkedAncestorEscape(t *testing.T) {
+	scope := t.TempDir()
+	if err := os.MkdirAll(filepath.Join(scope, "shared"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.MkdirAll(filepath.Join(scope, "private"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(scope, "private", "secret.txt"), []byte("secret"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(scope, "shared", "ok.txt"), []byte("ok"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Symlink(filepath.Join(scope, "private"), filepath.Join(scope, "shared", "link")); err != nil {
+		t.Skipf("cannot create symlink: %v", err)
+	}
+
+	// Filesystem scoped to the shared directory, as a public share would be.
+	bfs := afero.NewBasePathFs(afero.NewOsFs(), filepath.Join(scope, "shared"))
+
+	if _, err := stat(&FileOptions{Fs: bfs, Path: "/link/secret.txt"}); !os.IsPermission(err) {
+		t.Fatalf("expected permission error for linked-ancestor escape, got %v", err)
+	}
+	if _, err := stat(&FileOptions{Fs: bfs, Path: "/ok.txt"}); err != nil {
+		t.Fatalf("expected in-scope file to be served, got %v", err)
+	}
 }

http/public_symlink_test.go

@@ -0,0 +1,166 @@
+package fbhttp
+
+import (
+	"archive/zip"
+	"bytes"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/asdine/storm/v3"
+	"github.com/spf13/afero"
+
+	"github.com/filebrowser/filebrowser/v2/settings"
+	"github.com/filebrowser/filebrowser/v2/share"
+	"github.com/filebrowser/filebrowser/v2/storage"
+	"github.com/filebrowser/filebrowser/v2/storage/bolt"
+	"github.com/filebrowser/filebrowser/v2/users"
+)
+
+// symlinkShareStorage builds a storage whose single user is rooted at a real
+// on-disk scope containing a public share "/shared" with a symlinked
+// descendant "link -> ../private". Skips the test if symlinks are unavailable.
+func symlinkShareStorage(t *testing.T) *storage.Storage {
+	t.Helper()
+	scope := t.TempDir()
+	if err := os.MkdirAll(filepath.Join(scope, "shared"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.MkdirAll(filepath.Join(scope, "private"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(scope, "private", "secret.txt"), []byte("symlink-secret"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Symlink(filepath.Join(scope, "private"), filepath.Join(scope, "shared", "link")); err != nil {
+		t.Skipf("cannot create symlink on this platform: %v", err)
+	}
+
+	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.Share.Save(&share.Link{Hash: "h", UserID: 1, Path: "/shared"}); err != nil {
+		t.Fatalf("failed to save share: %v", err)
+	}
+	if err := st.Users.Save(&users.User{
+		Username: "username",
+		Password: "pw",
+		Perm:     users.Permissions{Share: true, Download: true},
+	}); err != nil {
+		t.Fatalf("failed to save user: %v", err)
+	}
+	if err := st.Settings.Save(&settings.Settings{Key: []byte("key")}); err != nil {
+		t.Fatalf("failed to save settings: %v", err)
+	}
+	st.Users = &customFSUser{
+		Store: st.Users,
+		fs:    afero.NewBasePathFs(afero.NewOsFs(), scope),
+	}
+	return st
+}
+
+// Reproduces GHSA-hf77-9m7w-fq8q: a public directory share whose subtree
+// contains a symlink to a directory outside the share. Requesting a regular
+// file behind that linked ancestor must NOT disclose its contents.
+func TestPublicShareSymlinkDescendantDisclosure(t *testing.T) {
+	cases := map[string]struct {
+		handler handleFunc
+		path    string
+	}{
+		"direct file download via dl handler": {handler: publicDlHandler, path: "h/link/secret.txt"},
+		"share info via share handler":        {handler: publicShareHandler, path: "h/link/secret.txt"},
+		"listing of linked dir":               {handler: publicShareHandler, path: "h/link/"},
+	}
+
+	for name, tc := range cases {
+		tc := tc
+		t.Run(name, func(t *testing.T) {
+			st := symlinkShareStorage(t)
+
+			req := newHTTPRequest(t, func(r *http.Request) { r.URL.Path = tc.path })
+			recorder := httptest.NewRecorder()
+			handler := handle(tc.handler, "", st, &settings.Server{})
+			handler.ServeHTTP(recorder, req)
+
+			result := recorder.Result()
+			defer result.Body.Close()
+			body, _ := io.ReadAll(result.Body)
+
+			t.Logf("status=%d body=%q", result.StatusCode, string(body))
+			if result.StatusCode == http.StatusOK {
+				t.Errorf("VULNERABLE: leaked path outside share (status 200, body=%q)", string(body))
+			}
+		})
+	}
+}
+
+// The listing of the public share root must omit the escaping symlink "link"
+// entirely (no target metadata leak).
+func TestPublicShareSymlinkListingOmitsEscapingLink(t *testing.T) {
+	st := symlinkShareStorage(t)
+
+	req := newHTTPRequest(t, func(r *http.Request) { r.URL.Path = "h/" })
+	recorder := httptest.NewRecorder()
+	handler := handle(publicShareHandler, "", st, &settings.Server{})
+	handler.ServeHTTP(recorder, req)
+
+	result := recorder.Result()
+	defer result.Body.Close()
+	body, _ := io.ReadAll(result.Body)
+	if result.StatusCode != http.StatusOK {
+		t.Fatalf("share root listing failed: status=%d body=%q", result.StatusCode, string(body))
+	}
+	if strings.Contains(string(body), "\"link\"") {
+		t.Errorf("VULNERABLE: listing exposes escaping symlink: %s", string(body))
+	}
+}
+
+// Reproduces the archive variant of GHSA-hf77-9m7w-fq8q: downloading the whole
+// public share as a zip must not pull in files reached through a symlinked
+// descendant.
+func TestPublicShareSymlinkArchiveDisclosure(t *testing.T) {
+	st := symlinkShareStorage(t)
+
+	// Request the whole share root as an archive.
+	req := newHTTPRequest(t, func(r *http.Request) { r.URL.Path = "h/" })
+	recorder := httptest.NewRecorder()
+	handler := handle(publicDlHandler, "", st, &settings.Server{})
+	handler.ServeHTTP(recorder, req)
+
+	result := recorder.Result()
+	defer result.Body.Close()
+	body, _ := io.ReadAll(result.Body)
+	if result.StatusCode != http.StatusOK {
+		t.Fatalf("archive request failed: status=%d body=%q", result.StatusCode, string(body))
+	}
+
+	zr, err := zip.NewReader(bytes.NewReader(body), int64(len(body)))
+	if err != nil {
+		t.Fatalf("failed to read zip: %v", err)
+	}
+	for _, f := range zr.File {
+		if strings.Contains(f.Name, "secret.txt") {
+			t.Errorf("VULNERABLE: archive includes file behind symlinked descendant: %q", f.Name)
+		}
+		rc, err := f.Open()
+		if err != nil {
+			continue
+		}
+		content, _ := io.ReadAll(rc)
+		rc.Close()
+		if bytes.Contains(content, []byte("symlink-secret")) {
+			t.Errorf("VULNERABLE: archive entry %q leaks out-of-scope content", f.Name)
+		}
+	}
+}

http/public_test.go

@@ -220,7 +220,13 @@ func TestPublicShareHandlerRules(t *testing.T) {
 				t.Fatalf("failed to save settings: %v", err)
 			}
 
-			fs := afero.NewMemMapFs()
+			fs := afero.NewBasePathFs(afero.NewOsFs(), t.TempDir())
+			if err := fs.MkdirAll("/projects/private", 0o755); err != nil {
+				t.Fatalf("failed to create private dir: %v", err)
+			}
+			if err := fs.MkdirAll("/projects/public", 0o755); err != nil {
+				t.Fatalf("failed to create public dir: %v", err)
+			}
 			if err := afero.WriteFile(fs, "/projects/private/secret.txt", []byte("top secret"), 0o600); err != nil {
 				t.Fatalf("failed to write secret file: %v", err)
 			}

http/raw.go

@@ -115,6 +115,10 @@ func getFiles(d *data, path, commonPath string) ([]archives.FileInfo, error) {
 		return nil, nil
 	}
 
+	if ok, err := files.WithinScope(d.user.Fs, path); err != nil || !ok {
+		return nil, nil
+	}
+
 	info, err := d.user.Fs.Stat(path)
 	if err != nil {
 		return nil, err

http/tus_handlers.go

@@ -44,6 +44,11 @@ func tusPostHandler(cache UploadCache) handleFunc {
 		if !d.user.Perm.Create || !d.Check(r.URL.Path) {
 			return http.StatusForbidden, nil
 		}
+
+		if ok, scopeErr := files.WithinScope(d.user.Fs, r.URL.Path); scopeErr != nil || !ok {
+			return http.StatusForbidden, nil
+		}
+
 		file, err := files.NewFileInfo(&files.FileOptions{
 			Fs:         d.user.Fs,
 			Path:       r.URL.Path,
@@ -161,6 +166,11 @@ func tusPatchHandler(cache UploadCache) handleFunc {
 		if r.Header.Get("Content-Type") != "application/offset+octet-stream" {
 			return http.StatusUnsupportedMediaType, nil
 		}
+		// Defense in depth: refuse to write through a symlink that escapes the
+		// scope, in case the target path is (or sits behind) an escaping link.
+		if ok, scopeErr := files.WithinScope(d.user.Fs, r.URL.Path); scopeErr != nil || !ok {
+			return http.StatusForbidden, nil
+		}
 
 		uploadOffset, err := getUploadOffset(r)
 		if err != nil {

http/tus_symlink_test.go

@@ -0,0 +1,118 @@
+package fbhttp
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/asdine/storm/v3"
+	"github.com/golang-jwt/jwt/v5"
+	"github.com/spf13/afero"
+
+	"github.com/filebrowser/filebrowser/v2/settings"
+	"github.com/filebrowser/filebrowser/v2/storage/bolt"
+	"github.com/filebrowser/filebrowser/v2/users"
+)
+
+// Reproduces the TUS write vector of GHSA-v9g6-9pp4-3w22: a scoped user must
+// not be able to create or write files through a symlinked directory that
+// escapes their scope.
+func TestTusHandlersRejectSymlinkScopeEscape(t *testing.T) {
+	root := t.TempDir()
+	userScope := filepath.Join(root, "user")
+	outside := filepath.Join(root, "otheruser")
+	for _, d := range []string{userScope, outside} {
+		if err := os.MkdirAll(d, 0o755); err != nil {
+			t.Fatal(err)
+		}
+	}
+	// A directory symlink inside the user's scope pointing outside it.
+	if err := os.Symlink(outside, filepath.Join(userScope, "escape_link")); err != nil {
+		t.Skipf("cannot create symlink: %v", err)
+	}
+
+	key := []byte("test-signing-key")
+
+	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.Users.Save(&users.User{
+		Username: "u",
+		Password: "pw",
+		Perm:     users.Permissions{Create: true, Modify: true},
+	}); err != nil {
+		t.Fatalf("failed to save user: %v", err)
+	}
+	if err := st.Settings.Save(&settings.Settings{Key: key}); err != nil {
+		t.Fatalf("failed to save settings: %v", err)
+	}
+	st.Users = &customFSUser{
+		Store: st.Users,
+		fs:    afero.NewBasePathFs(afero.NewOsFs(), userScope),
+	}
+
+	// Forge a valid auth token for user ID 1.
+	claims := &authToken{
+		User: userInfo{ID: 1, Username: "u", Perm: users.Permissions{Create: true, Modify: true}},
+		RegisteredClaims: jwt.RegisteredClaims{
+			IssuedAt:  jwt.NewNumericDate(time.Now().Add(-time.Minute)),
+			ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Hour)),
+		},
+	}
+	signed, err := jwt.NewWithClaims(jwt.SigningMethodHS256, claims).SignedString(key)
+	if err != nil {
+		t.Fatalf("failed to sign token: %v", err)
+	}
+
+	cases := map[string]struct {
+		method  string
+		handler handleFunc
+		headers map[string]string
+	}{
+		"POST create through symlinked dir": {
+			method:  http.MethodPost,
+			handler: tusPostHandler(newMemoryUploadCache()),
+			headers: map[string]string{"Upload-Length": "20"},
+		},
+		"PATCH write through symlinked dir": {
+			method:  http.MethodPatch,
+			handler: tusPatchHandler(newMemoryUploadCache()),
+			headers: map[string]string{"Content-Type": "application/offset+octet-stream", "Upload-Offset": "0"},
+		},
+	}
+
+	for name, tc := range cases {
+		tc := tc
+		t.Run(name, func(t *testing.T) {
+			req, err := http.NewRequest(tc.method, "escape_link/injected.txt", http.NoBody)
+			if err != nil {
+				t.Fatal(err)
+			}
+			req.Header.Set("X-Auth", signed)
+			for k, v := range tc.headers {
+				req.Header.Set(k, v)
+			}
+
+			recorder := httptest.NewRecorder()
+			handler := handle(tc.handler, "", st, &settings.Server{})
+			handler.ServeHTTP(recorder, req)
+
+			if recorder.Code != http.StatusForbidden {
+				t.Errorf("expected 403, got %d", recorder.Code)
+			}
+			if _, statErr := os.Stat(filepath.Join(outside, "injected.txt")); statErr == nil {
+				t.Errorf("VULNERABLE: file was created outside the user's scope")
+			}
+		})
+	}
+}