fix: harden request handling for files, search, proxy and SSO

Co-authored-by: PIKACHUIM <PIKACHUIM@users.noreply.github.com>

server/handles/fsmanage.go

@@ -113,6 +113,10 @@ func FsMove(c *gin.Context) {
 		srcDir += "/"
 	}
 	for i, name := range req.Names {
+		if err := checkRelativePath(name); err != nil {
+			common.ErrorResp(c, err, 403)
+			return
+		}
 		// ensure req.Names is not a relative path
 		srcPath := stdpath.Join(srcDir, name)
 		if !strings.HasPrefix(srcPath+"/", srcDir) {
@@ -216,6 +220,10 @@ func FsCopy(c *gin.Context) {
 		srcDir += "/"
 	}
 	for i, name := range req.Names {
+		if err := checkRelativePath(name); err != nil {
+			common.ErrorResp(c, err, 403)
+			return
+		}
 		// ensure req.Names is not a relative path
 		srcPath := stdpath.Join(srcDir, name)
 		if !strings.HasPrefix(srcPath+"/", srcDir) {
@@ -373,6 +381,10 @@ func FsRemove(c *gin.Context) {
 		reqPath += "/"
 	}
 	for i, name := range req.Names {
+		if err := checkRelativePath(name); err != nil {
+			common.ErrorResp(c, err, 403)
+			return
+		}
 		fullPath := stdpath.Join(reqPath, name)
 		if !strings.HasPrefix(fullPath+"/", reqPath) {
 			req.Names[i] = ""

server/handles/fsmanage_test.go

@@ -0,0 +1,126 @@
+package handles
+
+import (
+	"bytes"
+	"context"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	_ "github.com/OpenListTeam/OpenList/v4/drivers/local"
+	"github.com/OpenListTeam/OpenList/v4/internal/conf"
+	"github.com/OpenListTeam/OpenList/v4/internal/db"
+	"github.com/OpenListTeam/OpenList/v4/internal/model"
+	"github.com/OpenListTeam/OpenList/v4/internal/op"
+	"github.com/OpenListTeam/OpenList/v4/pkg/utils"
+	"github.com/gin-gonic/gin"
+	"github.com/glebarez/sqlite"
+	"gorm.io/gorm"
+)
+
+func setupBackslashTraversalTest(t *testing.T, root string, permission int32) *model.User {
+	t.Helper()
+	database, err := gorm.Open(sqlite.Open("file:"+t.Name()+"?mode=memory&cache=shared"), &gorm.Config{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	conf.Conf = conf.DefaultConfig(t.TempDir())
+	db.Init(database)
+	addition, err := utils.Json.MarshalToString(map[string]string{"root_folder_path": root})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err = op.CreateStorage(context.Background(), model.Storage{
+		Driver: "Local", MountPath: "/", Addition: addition,
+	}); err != nil {
+		t.Fatal(err)
+	}
+	return &model.User{
+		Username: "restricted-user", BasePath: "/team/a", Role: model.GENERAL,
+		Permission: permission,
+	}
+}
+
+func prepareBackslashTraversalFs(t *testing.T) (root string, secretPath string) {
+	t.Helper()
+	root = t.TempDir()
+	if err := os.MkdirAll(filepath.Join(root, "team", "a", "writable"), 0o700); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.MkdirAll(filepath.Join(root, "team", "ab"), 0o700); err != nil {
+		t.Fatal(err)
+	}
+	secretPath = filepath.Join(root, "team", "ab", "secret.txt")
+	if err := os.WriteFile(secretPath, []byte("synthetic-secret"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	return root, secretPath
+}
+
+func invokeHandler(t *testing.T, user *model.User, payload any, handler gin.HandlerFunc) *httptest.ResponseRecorder {
+	t.Helper()
+	body, err := json.Marshal(payload)
+	if err != nil {
+		t.Fatal(err)
+	}
+	recorder := httptest.NewRecorder()
+	ctx, _ := gin.CreateTestContext(recorder)
+	req := httptest.NewRequest(http.MethodPost, "/api/fs/remove", bytes.NewReader(body))
+	req.Header.Set("Content-Type", "application/json")
+	req = req.WithContext(context.WithValue(req.Context(), conf.UserKey, user))
+	ctx.Request = req
+	handler(ctx)
+	return recorder
+}
+
+func TestFsRemoveRejectsBackslashTraversal(t *testing.T) {
+	gin.SetMode(gin.TestMode)
+	root, secretPath := prepareBackslashTraversalFs(t)
+	user := setupBackslashTraversalTest(t, root, 1<<3|1<<7)
+
+	for _, name := range []string{"../../ab/secret.txt", `..\..\ab\secret.txt`} {
+		recorder := invokeHandler(t, user, map[string]any{"dir": "/writable", "names": []string{name}}, FsRemove)
+		if recorder.Code != http.StatusOK || !strings.Contains(recorder.Body.String(), `"code":403`) {
+			t.Fatalf("payload %q: got status=%d body=%s, want 403", name, recorder.Code, recorder.Body.String())
+		}
+		if _, err := os.Stat(secretPath); err != nil {
+			t.Fatalf("payload %q deleted sibling file: %v", name, err)
+		}
+	}
+}
+
+func TestFsMoveRejectsBackslashTraversal(t *testing.T) {
+	gin.SetMode(gin.TestMode)
+	root, secretPath := prepareBackslashTraversalFs(t)
+	user := setupBackslashTraversalTest(t, root, 1<<3|1<<5)
+
+	recorder := invokeHandler(t, user, map[string]any{
+		"src_dir": "/writable", "dst_dir": "/", "names": []string{`..\..\ab\secret.txt`},
+	}, FsMove)
+	if recorder.Code != http.StatusOK || !strings.Contains(recorder.Body.String(), `"code":403`) {
+		t.Fatalf("got status=%d body=%s, want 403", recorder.Code, recorder.Body.String())
+	}
+	if _, err := os.Stat(secretPath); err != nil {
+		t.Fatalf("backslash traversal moved sibling file: %v", err)
+	}
+}
+
+func TestFsCopyRejectsBackslashTraversal(t *testing.T) {
+	gin.SetMode(gin.TestMode)
+	root, secretPath := prepareBackslashTraversalFs(t)
+	user := setupBackslashTraversalTest(t, root, 1<<3|1<<6)
+
+	recorder := invokeHandler(t, user, map[string]any{
+		"src_dir": "/writable", "dst_dir": "/", "names": []string{`..\..\ab\secret.txt`},
+	}, FsCopy)
+	if recorder.Code != http.StatusOK || !strings.Contains(recorder.Body.String(), `"code":403`) {
+		t.Fatalf("got status=%d body=%s, want 403", recorder.Code, recorder.Body.String())
+	}
+	if _, err := os.Stat(secretPath); err != nil {
+		t.Fatalf("backslash traversal affected sibling file: %v", err)
+	}
+}

server/handles/search.go

@@ -44,14 +44,7 @@ func Search(c *gin.Context) {
 		return
 	}
 	nodes, total, err := search.SearchFiltered(c, req.SearchReq, func(node model.SearchNode) bool {
-		if !utils.IsSubPath(user.BasePath, node.Parent) {
-			return false
-		}
-		meta, err := op.GetNearestMeta(node.Parent)
-		if err != nil && !errors.Is(errors.Cause(err), errs.MetaNotFound) {
-			return false
-		}
-		return common.CanAccess(user, meta, path.Join(node.Parent, node.Name), req.Password)
+		return isSearchNodeAccessible(user, node, req.Password, op.GetNearestMeta)
 	})
 	if err != nil {
 		common.ErrorResp(c, err, 500)
@@ -63,6 +56,22 @@ func Search(c *gin.Context) {
 	})
 }
 
+func isSearchNodeAccessible(user *model.User, node model.SearchNode, password string, resolveMeta func(string) (*model.Meta, error)) bool {
+	if !utils.IsSubPath(user.BasePath, node.Parent) {
+		return false
+	}
+	nodePath := path.Join(node.Parent, node.Name)
+	metaPath := node.Parent
+	if node.IsDir {
+		metaPath = nodePath
+	}
+	meta, err := resolveMeta(metaPath)
+	if err != nil && !errors.Is(errors.Cause(err), errs.MetaNotFound) {
+		return false
+	}
+	return common.CanAccess(user, meta, nodePath, password)
+}
+
 func nodeToSearchResp(node model.SearchNode) SearchResp {
 	return SearchResp{
 		SearchNode: node,

server/handles/search_test.go

@@ -0,0 +1,78 @@
+package handles
+
+import (
+	"path"
+	"testing"
+
+	"github.com/OpenListTeam/OpenList/v4/internal/errs"
+	"github.com/OpenListTeam/OpenList/v4/internal/model"
+)
+
+func fakeResolveMeta(metas map[string]*model.Meta) func(string) (*model.Meta, error) {
+	return func(p string) (*model.Meta, error) {
+		for {
+			if meta, ok := metas[p]; ok {
+				return meta, nil
+			}
+			if p == "/" {
+				return nil, errs.MetaNotFound
+			}
+			p = path.Dir(p)
+		}
+	}
+}
+
+func TestIsSearchNodeAccessible(t *testing.T) {
+	tests := []struct {
+		name         string
+		metas        map[string]*model.Meta
+		node         model.SearchNode
+		want         bool
+		wantMetaPath string
+	}{
+		{
+			name:         "restricted directory",
+			metas:        map[string]*model.Meta{"/private": {Path: "/private", ReadUsers: []uint{1}}},
+			node:         model.SearchNode{Parent: "/", Name: "private", IsDir: true},
+			want:         false,
+			wantMetaPath: "/private",
+		},
+		{
+			name:         "restricted sub directory",
+			metas:        map[string]*model.Meta{"/private": {Path: "/private", ReadUsers: []uint{1}, ReadUsersSub: true}},
+			node:         model.SearchNode{Parent: "/private", Name: "sub", IsDir: true},
+			want:         false,
+			wantMetaPath: "/private/sub",
+		},
+		{
+			name:         "file keeps parent scope",
+			metas:        map[string]*model.Meta{"/private": {Path: "/private", ReadUsers: []uint{1}}},
+			node:         model.SearchNode{Parent: "/private", Name: "a.txt", IsDir: false},
+			want:         true,
+			wantMetaPath: "/private",
+		},
+		{
+			name:         "outside base path",
+			node:         model.SearchNode{Parent: "/other", Name: "private", IsDir: true},
+			want:         false,
+			wantMetaPath: "",
+		},
+	}
+	user := &model.User{ID: 2, BasePath: "/"}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			resolve := fakeResolveMeta(tt.metas)
+			var gotMetaPath string
+			spy := func(p string) (*model.Meta, error) {
+				gotMetaPath = p
+				return resolve(p)
+			}
+			if got := isSearchNodeAccessible(user, tt.node, "", spy); got != tt.want {
+				t.Fatalf("isSearchNodeAccessible() = %v, want %v", got, tt.want)
+			}
+			if tt.wantMetaPath != "" && gotMetaPath != tt.wantMetaPath {
+				t.Fatalf("meta resolved at %q, want %q", gotMetaPath, tt.wantMetaPath)
+			}
+		})
+	}
+}

server/handles/ssologin.go

@@ -122,6 +122,53 @@ func generateSSOBindingToken(c *gin.Context, purpose, ssoID string) (string, err
 	return jwt.NewWithClaims(jwt.SigningMethodHS256, claims).SignedString(common.SecretKey)
 }
 
+// ssoTargetOrigin returns the origin that is allowed to receive the SSO result
+// via postMessage. It honours the operator-configured sso_postmessage_origin so
+// a frontend served from a different origin than the API can still receive the
+// result; otherwise it falls back to the API origin, or "/" to restrict
+// delivery to same-origin openers when that cannot be resolved.
+func ssoTargetOrigin(c *gin.Context) string {
+	if configured := setting.GetStr(conf.SSOPostMessageOrigin); configured != "" {
+		if u, err := url.Parse(configured); err == nil &&
+			(u.Scheme == "http" || u.Scheme == "https") &&
+			u.Host != "" && u.User == nil &&
+			(u.Path == "" || u.Path == "/") &&
+			u.RawQuery == "" && u.Fragment == "" {
+			return u.Scheme + "://" + u.Host
+		}
+	}
+	u, err := url.Parse(common.GetApiUrl(c))
+	if err != nil || u.Scheme == "" || u.Host == "" {
+		return "/"
+	}
+	return u.Scheme + "://" + u.Host
+}
+
+// ssoPostMessage hands the SSO result back to the window that started the login.
+// The target origin is pinned so that an arbitrary page cannot open the SSO
+// endpoint in a popup and read the payload out of the message event.
+func ssoPostMessage(c *gin.Context, payload map[string]string) {
+	data, err := utils.Json.MarshalToString(payload)
+	if err != nil {
+		common.ErrorResp(c, err, 500)
+		return
+	}
+	origin, err := utils.Json.MarshalToString(ssoTargetOrigin(c))
+	if err != nil {
+		common.ErrorResp(c, err, 500)
+		return
+	}
+	html := fmt.Sprintf(`<!DOCTYPE html>
+			<head></head>
+			<body>
+			<script>
+			if (window.opener) { window.opener.postMessage(%s, %s) }
+			window.close()
+			</script>
+			</body>`, data, origin)
+	c.Data(200, "text/html; charset=utf-8", []byte(html))
+}
+
 func ssoRedirectUri(c *gin.Context, useCompatibility bool, method string) string {
 	if useCompatibility {
 		return common.GetApiUrl(c) + "/api/auth/" + method
@@ -338,15 +385,7 @@ func OIDCLoginCallback(c *gin.Context) {
 			c.Redirect(302, common.GetApiUrl(c)+"/@manage?sso_id="+bindingProof)
 			return
 		}
-		html := fmt.Sprintf(`<!DOCTYPE html>
-				<head></head>
-				<body>
-				<script>
-				window.opener.postMessage({"sso_id": "%s"}, "*")
-				window.close()
-				</script>
-				</body>`, bindingProof)
-		c.Data(200, "text/html; charset=utf-8", []byte(html))
+		ssoPostMessage(c, map[string]string{"sso_id": bindingProof})
 		return
 	}
 	if method == "sso_get_token" {
@@ -367,15 +406,7 @@ func OIDCLoginCallback(c *gin.Context) {
 			c.Redirect(302, common.GetApiUrl(c)+"/@login?token="+token)
 			return
 		}
-		html := fmt.Sprintf(`<!DOCTYPE html>
-				<head></head>
-				<body>
-				<script>
-				window.opener.postMessage({"token":"%s"}, "*")
-				window.close()
-				</script>
-				</body>`, token)
-		c.Data(200, "text/html; charset=utf-8", []byte(html))
+		ssoPostMessage(c, map[string]string{"token": token})
 		return
 	}
 }
@@ -516,15 +547,7 @@ func SSOLoginCallback(c *gin.Context) {
 			c.Redirect(302, common.GetApiUrl(c)+"/@manage?sso_id="+bindingProof)
 			return
 		}
-		html := fmt.Sprintf(`<!DOCTYPE html>
-				<head></head>
-				<body>
-				<script>
-				window.opener.postMessage({"sso_id": "%s"}, "*")
-				window.close()
-				</script>
-				</body>`, bindingProof)
-		c.Data(200, "text/html; charset=utf-8", []byte(html))
+		ssoPostMessage(c, map[string]string{"sso_id": bindingProof})
 		return
 	}
 	username := utils.Json.Get(resp.Body(), usernameField).ToString()
@@ -545,13 +568,5 @@ func SSOLoginCallback(c *gin.Context) {
 		c.Redirect(302, common.GetApiUrl(c)+"/@login?token="+token)
 		return
 	}
-	html := fmt.Sprintf(`<!DOCTYPE html>
-							<head></head>
-							<body>
-							<script>
-							window.opener.postMessage({"token":"%s"}, "*")
-							window.close()
-							</script>
-							</body>`, token)
-	c.Data(200, "text/html; charset=utf-8", []byte(html))
+	ssoPostMessage(c, map[string]string{"token": token})
 }

server/handles/ssologin_origin_test.go

@@ -0,0 +1,102 @@
+package handles
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/OpenListTeam/OpenList/v4/internal/conf"
+	"github.com/OpenListTeam/OpenList/v4/internal/model"
+	"github.com/OpenListTeam/OpenList/v4/internal/op"
+	"github.com/gin-gonic/gin"
+)
+
+func ssoTestContext(apiUrl string) (*gin.Context, *httptest.ResponseRecorder) {
+	gin.SetMode(gin.TestMode)
+	rec := httptest.NewRecorder()
+	c, engine := gin.CreateTestContext(rec)
+	// Matches server.Init, which is what lets GetApiUrl reach the value the
+	// middleware stored on the request context.
+	engine.ContextWithFallback = true
+	req := httptest.NewRequest(http.MethodGet, "/api/auth/sso?method=sso_get_token", nil)
+	if apiUrl != "" {
+		req = req.WithContext(context.WithValue(req.Context(), conf.ApiUrlKey, apiUrl))
+	}
+	c.Request = req
+	// Keep setting lookups off the (uninitialised) database: ssoTargetOrigin
+	// reads sso_postmessage_origin through the setting cache.
+	op.Cache.SetSetting(conf.SSOPostMessageOrigin, &model.SettingItem{
+		Key:   conf.SSOPostMessageOrigin,
+		Value: "",
+	})
+	return c, rec
+}
+
+// A page that opens the SSO endpoint in a popup must not be able to read the
+// token: the postMessage target origin has to name the site, never "*".
+func TestSSOPostMessagePinsTargetOrigin(t *testing.T) {
+	c, rec := ssoTestContext("https://openlist.example.com/base")
+	ssoPostMessage(c, map[string]string{"token": "secret-token"})
+
+	body := rec.Body.String()
+	if strings.Contains(body, `"*"`) || strings.Contains(body, `, '*'`) {
+		t.Fatalf("wildcard target origin present in response:\n%s", body)
+	}
+	if !strings.Contains(body, `"https://openlist.example.com"`) {
+		t.Errorf("expected the site origin as target, got:\n%s", body)
+	}
+	if !strings.Contains(body, "secret-token") {
+		t.Errorf("payload should still reach a legitimate opener, got:\n%s", body)
+	}
+}
+
+// A frontend served from a different origin than the API needs the operator to
+// be able to point the target at the frontend origin. The configured origin
+// must win over the API origin.
+func TestSSOPostMessageUsesConfiguredOrigin(t *testing.T) {
+	c, rec := ssoTestContext("https://api.example.com/base")
+	op.Cache.SetSetting(conf.SSOPostMessageOrigin, &model.SettingItem{
+		Key:   conf.SSOPostMessageOrigin,
+		Value: "https://frontend.example.com",
+	})
+	defer op.Cache.ClearAll()
+
+	ssoPostMessage(c, map[string]string{"token": "secret-token"})
+
+	body := rec.Body.String()
+	if !strings.Contains(body, `"https://frontend.example.com"`) {
+		t.Errorf("expected the configured origin as target, got:\n%s", body)
+	}
+}
+
+// If the site URL cannot be resolved the fallback must tighten delivery to
+// same-origin openers, not widen it back to every origin.
+func TestSSOPostMessageFallsBackToSameOrigin(t *testing.T) {
+	c, rec := ssoTestContext("")
+	ssoPostMessage(c, map[string]string{"token": "secret-token"})
+
+	body := rec.Body.String()
+	if strings.Contains(body, `"*"`) {
+		t.Fatalf("fallback must not be a wildcard origin:\n%s", body)
+	}
+	if !strings.Contains(body, `"/"`) {
+		t.Errorf(`expected "/" fallback origin, got:\n%s`, body)
+	}
+}
+
+// userID comes from the identity provider, so it must be encoded rather than
+// interpolated into the JS string literal it used to land in.
+func TestSSOPostMessageEscapesProviderControlledValue(t *testing.T) {
+	c, rec := ssoTestContext("https://openlist.example.com")
+	ssoPostMessage(c, map[string]string{"sso_id": `"});alert(document.domain);//`})
+
+	body := rec.Body.String()
+	if strings.Contains(body, `alert(document.domain)`) && !strings.Contains(body, `\"`) {
+		t.Fatalf("provider value was not escaped:\n%s", body)
+	}
+	if !strings.Contains(body, `\"});alert`) {
+		t.Errorf("expected the injected quote to be escaped, got:\n%s", body)
+	}
+}