Skip to content

Commit e9a7d7c

Browse files
committed
fix(security): enforce user base path on all request paths
JoinBasePath returned the request path unchanged whenever it started with "/". Since FixAndCleanPath always prepends "/", that branch matched every request, so the user's base path was never applied and only the exact path "/" was rebased (by User.JoinPath). A user restricted to a base path could therefore address any path in the tree: with the built-in guest/general role scope of "/", a guest confined to /public could list and download other storages by requesting their absolute paths. It also broke normal navigation for such users, since entering a subfolder resolved to the real root instead of a path under the base. Restore the join. Regression introduced in 6b2d81e (#9249). Covered by tests asserting that non-root request paths are rebased and that relative segments are still rejected.
1 parent aead76e commit e9a7d7c

2 files changed

Lines changed: 33 additions & 6 deletions

File tree

pkg/utils/path.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -89,12 +89,6 @@ func JoinBasePath(basePath, reqPath string) (string, error) {
8989
return "", errs.RelativePath
9090
}
9191

92-
reqPath = FixAndCleanPath(reqPath)
93-
94-
if strings.HasPrefix(reqPath, "/") {
95-
return reqPath, nil
96-
}
97-
9892
return stdpath.Join(FixAndCleanPath(basePath), FixAndCleanPath(reqPath)), nil
9993
}
10094

pkg/utils/path_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,3 +66,36 @@ func TestJoinUnderBase(t *testing.T) {
6666
t.Fatalf("expected nested path to be rejected")
6767
}
6868
}
69+
70+
func TestJoinBasePath(t *testing.T) {
71+
// A user's base path confines every request path, not just "/".
72+
datas := []struct {
73+
basePath string
74+
reqPath string
75+
want string
76+
}{
77+
{"/public", "/", "/public"},
78+
{"/public", "/sub", "/public/sub"},
79+
{"/public", "sub", "/public/sub"},
80+
{"/public", "/sub/file.txt", "/public/sub/file.txt"},
81+
{"/public", "/other_storage", "/public/other_storage"},
82+
{"/", "/sub", "/sub"},
83+
{"", "/sub", "/sub"},
84+
}
85+
for _, d := range datas {
86+
got, err := JoinBasePath(d.basePath, d.reqPath)
87+
if err != nil {
88+
t.Fatalf("JoinBasePath(%q, %q) error: %v", d.basePath, d.reqPath, err)
89+
}
90+
if got != d.want {
91+
t.Errorf("JoinBasePath(%q, %q) = %q, want %q", d.basePath, d.reqPath, got, d.want)
92+
}
93+
}
94+
95+
// relative segments must still be rejected
96+
for _, reqPath := range []string{"..", "../x", "/x/..", "/x/../y"} {
97+
if _, err := JoinBasePath("/public", reqPath); err == nil {
98+
t.Errorf("JoinBasePath(%q, %q) expected error, got nil", "/public", reqPath)
99+
}
100+
}
101+
}

0 commit comments

Comments
 (0)