filer: scope TUS HEAD/PATCH/DELETE against the session target path (#10309)

* filer: scope TUS HEAD/PATCH/DELETE against the session target path

checkTusJwtAuthorization only populated the scoped-path list for POST, so
a prefix-restricted token was scope-checked at session creation but not on
HEAD/PATCH/DELETE, which act on an existing session addressed by id. A
low-privilege tenant holding a valid write token who learns another
upload's session id could PATCH attacker bytes into that session, or
DELETE/HEAD it, landing content at a target path its own AllowedPrefixes
forbids. The session id is unguessable to an unauthenticated attacker but
is not an authorization boundary against a legitimate tenant.

Resolve the effective target for HEAD/PATCH/DELETE from the session's
stored TargetPath and scope the prefix check against it, the same way POST
scopes the create target. The scoped paths are resolved lazily so the
session read runs only for a prefix-restricted token; unrestricted tokens
and deployments without a signing key touch no extra state. An unknown
session stays unscoped so the handler still answers 404.

* filer: fail closed when a TUS session target cannot be resolved

The lazy scope resolver swallowed every readTusSessionInfo error and left
the request unscoped, so a filer read error or a corrupt session .info
would authorize a prefix-restricted token against a target the server
never resolved. Only a genuinely missing session should stay unscoped (so
the handler answers 404); any other failure now propagates and denies the
request. checkJwtAuthorizationScoped's resolver returns an error and a
failed resolution is treated as not-authorized.

* filer: fold the scoped-path resolver into checkJwtAuthorization

checkJwtAuthorization now takes the lazy resolver directly instead of a
separate checkJwtAuthorizationScoped wrapper, and the surrounding comments
are trimmed to the load-bearing why.

weed/server/filer_server_handlers.go

@@ -216,15 +216,16 @@ func (fs *FilerServer) maybeCheckJwtAuthorization(r *http.Request, isWrite bool)
 		return true
 	}
 
-	return fs.checkJwtAuthorization(r, isWrite, jwtScopedRequestPaths(r))
+	return fs.checkJwtAuthorization(r, isWrite, func() ([]string, error) { return jwtScopedRequestPaths(r), nil })
 }
 
 // checkJwtAuthorization verifies the request carries a valid filer JWT for the
-// requested access level and, for prefix-restricted tokens, that every path in
-// scopedPaths falls within the token's AllowedPrefixes. Callers whose write
-// target is not r.URL.Path — the TUS handler, whose URL points at the /.tus
-// route — pass the resolved filer path(s) here instead.
-func (fs *FilerServer) checkJwtAuthorization(r *http.Request, isWrite bool, scopedPaths []string) bool {
+// requested access level and, for prefix-restricted tokens, that every resolved
+// path falls within AllowedPrefixes. resolveScopedPaths is invoked only for a
+// prefix-restricted token, so callers that read extra state to name their target
+// (the TUS handler reads the session's stored path) pay for it only when it is
+// consulted; a resolution error denies the request.
+func (fs *FilerServer) checkJwtAuthorization(r *http.Request, isWrite bool, resolveScopedPaths func() ([]string, error)) bool {
 
 	var signingKey security.SigningKey
 
@@ -263,9 +264,11 @@ func (fs *FilerServer) checkJwtAuthorization(r *http.Request, isWrite bool, scop
 	}
 
 	if len(claims.AllowedPrefixes) > 0 {
-		// Copy and move name their source via a query parameter, not r.URL.Path.
-		// Scope every path the request reads or relocates, or a prefix-restricted
-		// token could reach data outside its allowed subtree.
+		scopedPaths, err := resolveScopedPaths()
+		if err != nil {
+			glog.V(1).Infof("jwt scope resolution failed from %s: %v", r.RemoteAddr, err)
+			return false
+		}
 		for _, p := range scopedPaths {
 			if !anyComponentPrefixMatches(claims.AllowedPrefixes, p) {
 				glog.V(1).Infof("jwt path not allowed from %s: %v", r.RemoteAddr, p)

weed/server/filer_server_tus_handlers.go

@@ -53,8 +53,7 @@ func (fs *FilerServer) tusHandler(w http.ResponseWriter, r *http.Request) {
 	// Check if this is an upload location (contains upload ID after {tusPrefix}/.uploads/)
 	uploadsPrefix := tusPrefix + "/.uploads/"
 	if strings.HasPrefix(reqPath, uploadsPrefix) {
-		uploadID := strings.TrimPrefix(reqPath, uploadsPrefix)
-		uploadID = strings.Split(uploadID, "/")[0] // Get just the ID, not any trailing path
+		uploadID := fs.tusUploadID(reqPath)
 
 		switch r.Method {
 		case http.MethodHead:
@@ -81,21 +80,53 @@ func (fs *FilerServer) tusHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 // checkTusJwtAuthorization enforces filer JWT authorization for a TUS request.
-// HEAD reads the current offset (read access); POST/PATCH/DELETE mutate the
-// namespace (write access). Only creation (POST) names a new target path, taken
-// from the request URL, so prefix-restricted tokens are scoped against that
-// resolved filer path rather than the /.tus route. HEAD/PATCH/DELETE act on an
-// existing session addressed by an unguessable id, whose target was already
-// scope-checked at creation, so no further path scoping is applied to them.
+// HEAD is a read; POST/PATCH/DELETE are writes.
 func (fs *FilerServer) checkTusJwtAuthorization(r *http.Request) bool {
 	isWrite := r.Method != http.MethodHead
-	var scopedPaths []string
-	if r.Method == http.MethodPost {
+	return fs.checkJwtAuthorization(r, isWrite, func() ([]string, error) {
+		return fs.tusScopedPaths(r)
+	})
+}
+
+// tusScopedPaths returns the filer path(s) a prefix-restricted token must be
+// allowed to reach. POST names its target in the URL; HEAD/PATCH/DELETE act on an
+// existing session whose stored TargetPath must be re-checked, since the session
+// id is not an authorization boundary against a tenant who already holds a token.
+// A missing session stays unscoped (handler answers 404); any other lookup error
+// is returned so the caller fails closed.
+func (fs *FilerServer) tusScopedPaths(r *http.Request) ([]string, error) {
+	switch r.Method {
+	case http.MethodPost:
 		if target := fs.tusTargetPath(r); target != "" && target != "/" {
-			scopedPaths = append(scopedPaths, target)
+			return []string{target}, nil
+		}
+	case http.MethodHead, http.MethodPatch, http.MethodDelete:
+		uploadID := fs.tusUploadID(r.URL.Path)
+		if uploadID == "" {
+			return nil, nil
+		}
+		session, err := fs.readTusSessionInfo(r.Context(), uploadID)
+		if err != nil {
+			if errors.Is(err, filer_pb.ErrNotFound) {
+				return nil, nil
+			}
+			return nil, err
+		}
+		if target := session.TargetPath; target != "" && target != "/" {
+			return []string{target}, nil
 		}
 	}
-	return fs.checkJwtAuthorization(r, isWrite, scopedPaths)
+	return nil, nil
+}
+
+// tusUploadID extracts the session id from a {TusBasePath}/.uploads/{id} path, or
+// "" when the path is not an uploads route.
+func (fs *FilerServer) tusUploadID(reqPath string) string {
+	uploadsPrefix := fs.option.TusBasePath + "/.uploads/"
+	if !strings.HasPrefix(reqPath, uploadsPrefix) {
+		return ""
+	}
+	return strings.Split(strings.TrimPrefix(reqPath, uploadsPrefix), "/")[0]
 }
 
 // tusTargetPath resolves the filer path a TUS create request targets from the

weed/server/filer_server_tus_idor_test.go

@@ -0,0 +1,132 @@
+package weed_server
+
+import (
+	"context"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/seaweedfs/seaweedfs/weed/filer"
+	"github.com/seaweedfs/seaweedfs/weed/security"
+	"github.com/seaweedfs/seaweedfs/weed/util"
+)
+
+// newTusIDORTestServer builds a FilerServer backed by an in-memory store seeded
+// with TUS sessions (uploadID -> stored TargetPath), so the JWT check can resolve
+// a session's target the way the real handler does. The store is returned so a
+// test can seed additional (e.g. corrupt) session entries.
+func newTusIDORTestServer(t *testing.T, writeKey, readKey string, sessions map[string]string) (*FilerServer, *renameTestStore) {
+	t.Helper()
+	store := newRenameTestStore()
+	fs := &FilerServer{
+		filer:      newRenameTestFiler(store),
+		filerGuard: security.NewGuard(nil, writeKey, 0, readKey, 0),
+		option:     &FilerOption{TusBasePath: "/.tus"},
+	}
+	for uploadID, targetPath := range sessions {
+		data, err := json.Marshal(&TusSession{ID: uploadID, TargetPath: targetPath, Size: 46})
+		if err != nil {
+			t.Fatalf("marshal session %s: %v", uploadID, err)
+		}
+		if err := store.InsertEntry(context.Background(), &filer.Entry{FullPath: util.FullPath(fs.tusSessionInfoPath(uploadID)), Content: data}); err != nil {
+			t.Fatalf("seed session %s: %v", uploadID, err)
+		}
+	}
+	return fs, store
+}
+
+// TestFilerServer_checkTusJwtAuthorization_CrossPrefixSessionHijack reproduces
+// GHSA-99q7-x53r-6j4g: a prefix-restricted token acting on another tenant's
+// existing TUS session (HEAD/PATCH/DELETE) must be scoped against the session's
+// stored TargetPath, not authorized on signature and method alone.
+func TestFilerServer_checkTusJwtAuthorization_CrossPrefixSessionHijack(t *testing.T) {
+	const writeKey = "write-secret"
+	const readKey = "read-secret"
+
+	fs, store := newTusIDORTestServer(t, writeKey, readKey, map[string]string{
+		"victim-session": "/buckets/secret/victim.bin",
+		"own-session":    "/buckets/allowed/own.bin",
+	})
+
+	// A session whose .info is unreadable (corrupt JSON) must fail closed rather
+	// than authorize a prefix-restricted token against a target we cannot resolve.
+	if err := store.InsertEntry(context.Background(), &filer.Entry{
+		FullPath: util.FullPath(fs.tusSessionInfoPath("corrupt-session")),
+		Content:  []byte("{not valid json"),
+	}); err != nil {
+		t.Fatalf("seed corrupt session: %v", err)
+	}
+
+	attackerWrite := signFilerToken(t, writeKey, []string{"/buckets/allowed"}, nil)
+	attackerRead := signFilerToken(t, readKey, []string{"/buckets/allowed"}, nil)
+
+	tests := []struct {
+		name             string
+		method           string
+		path             string
+		token            string
+		expectAuthorized bool
+	}{
+		// The IDOR: a token scoped to /buckets/allowed must not act on a session
+		// whose target is /buckets/secret, regardless of the verb.
+		{"patch victim session denied", http.MethodPatch, "/.tus/.uploads/victim-session", attackerWrite, false},
+		{"delete victim session denied", http.MethodDelete, "/.tus/.uploads/victim-session", attackerWrite, false},
+		{"head victim session denied", http.MethodHead, "/.tus/.uploads/victim-session", attackerRead, false},
+
+		// The same token acting on its own in-prefix session is still allowed.
+		{"patch own session allowed", http.MethodPatch, "/.tus/.uploads/own-session", attackerWrite, true},
+		{"delete own session allowed", http.MethodDelete, "/.tus/.uploads/own-session", attackerWrite, true},
+		{"head own session allowed", http.MethodHead, "/.tus/.uploads/own-session", attackerRead, true},
+
+		// An unrestricted token (no AllowedPrefixes) keeps working and triggers no
+		// session lookup.
+		{"patch unrestricted allowed", http.MethodPatch, "/.tus/.uploads/victim-session", signFilerToken(t, writeKey, nil, nil), true},
+
+		// An unknown session leaves the request unscoped so the handler can answer
+		// 404, rather than being denied on a path that cannot be resolved.
+		{"patch unknown session allowed", http.MethodPatch, "/.tus/.uploads/does-not-exist", attackerWrite, true},
+
+		// A corrupt/unreadable session fails closed: the target cannot be resolved
+		// so a prefix-restricted token must be denied, not authorized.
+		{"patch corrupt session denied", http.MethodPatch, "/.tus/.uploads/corrupt-session", attackerWrite, false},
+		{"head corrupt session denied", http.MethodHead, "/.tus/.uploads/corrupt-session", attackerRead, false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			req := httptest.NewRequest(tt.method, tt.path, nil)
+			req.Header.Set("Authorization", "Bearer "+tt.token)
+			if got := fs.checkTusJwtAuthorization(req); got != tt.expectAuthorized {
+				t.Errorf("checkTusJwtAuthorization(%s %s) = %v, want %v", tt.method, tt.path, got, tt.expectAuthorized)
+			}
+		})
+	}
+}
+
+// TestFilerServer_tusHandler_CrossPrefixPatchRejected drives the full handler:
+// an attacker PATCH against another tenant's session must be rejected with 401
+// before any bytes are written to the session's target path.
+func TestFilerServer_tusHandler_CrossPrefixPatchRejected(t *testing.T) {
+	const writeKey = "write-secret"
+	const readKey = "read-secret"
+
+	fs, _ := newTusIDORTestServer(t, writeKey, readKey, map[string]string{
+		"victim-session": "/buckets/secret/victim.bin",
+	})
+	attacker := signFilerToken(t, writeKey, []string{"/buckets/allowed"}, nil)
+
+	req := httptest.NewRequest(http.MethodPatch, "/.tus/.uploads/victim-session", strings.NewReader("PWNED-BY-CROSS-PREFIX-TUS-SESSION-HIJACK"))
+	req.Header.Set("Tus-Resumable", TusVersion)
+	req.Header.Set("Upload-Offset", "0")
+	req.Header.Set("Content-Type", "application/offset+octet-stream")
+	req.Header.Set("Authorization", "Bearer "+attacker)
+
+	rec := httptest.NewRecorder()
+	fs.tusHandler(rec, req)
+
+	if rec.Code != http.StatusUnauthorized {
+		t.Errorf("cross-prefix PATCH = %d, want %d", rec.Code, http.StatusUnauthorized)
+	}
+}

weed/server/filer_server_tus_session.go

@@ -181,13 +181,14 @@ func (fs *FilerServer) saveTusSession(ctx context.Context, session *TusSession)
 	return nil
 }
 
-// getTusSession retrieves a TUS session by upload ID, including chunks from directory listing
-func (fs *FilerServer) getTusSession(ctx context.Context, uploadID string) (*TusSession, error) {
+// readTusSessionInfo reads and decodes a session's .info file without listing its
+// chunks, the cheap lookup the authorization check needs for the stored TargetPath.
+func (fs *FilerServer) readTusSessionInfo(ctx context.Context, uploadID string) (*TusSession, error) {
 	infoPath := util.FullPath(fs.tusSessionInfoPath(uploadID))
 	entry, err := fs.filer.FindEntry(ctx, infoPath)
 	if err != nil {
 		if err == filer_pb.ErrNotFound {
-			return nil, fmt.Errorf("TUS upload session not found: %s", uploadID)
+			return nil, fmt.Errorf("TUS upload session not found: %s: %w", uploadID, filer_pb.ErrNotFound)
 		}
 		return nil, fmt.Errorf("find session: %w", err)
 	}
@@ -196,6 +197,15 @@ func (fs *FilerServer) getTusSession(ctx context.Context, uploadID string) (*Tus
 	if err := json.Unmarshal(entry.Content, &session); err != nil {
 		return nil, fmt.Errorf("unmarshal session: %w", err)
 	}
+	return &session, nil
+}
+
+// getTusSession retrieves a TUS session by upload ID, including chunks from directory listing
+func (fs *FilerServer) getTusSession(ctx context.Context, uploadID string) (*TusSession, error) {
+	session, err := fs.readTusSessionInfo(ctx, uploadID)
+	if err != nil {
+		return nil, err
+	}
 
 	// Load chunks from directory listing with pagination (atomic read, no race condition)
 	sessionDirPath := util.FullPath(fs.tusSessionPath(uploadID))
@@ -248,7 +258,7 @@ func (fs *FilerServer) getTusSession(ctx context.Context, uploadID string) (*Tus
 		session.Offset = contiguousEnd
 	}
 
-	return &session, nil
+	return session, nil
 }
 
 // saveTusChunk stores the chunk info as a separate file entry