mirror of
https://github.com/gtsteffaniak/filebrowser.git
synced 2026-11-13 20:51:59 -05:00
Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
116a7c3677 |
No files matched your search
+1
-1
@@ -64,7 +64,7 @@ All notable changes to this project will be documented in this file. For commit
|
||||
- [Moderate] Authenticated `GET /api/resources/view` no longer accepts view tokens minted for a public share when the caller lacks view permission on the requested source (GHSA-wxj5-g3qp-g732) reporter @BlueN0r
|
||||
- [Low] SSE source-update and user-targeted delivery hold the client registry lock through nonblocking sends, preventing concurrent map mutation and send-on-closed-channel panics when Realtime clients connect, disconnect, or shut down (GHSA-p9wm-f5jj-75pj) reporter @tao0845
|
||||
- [High] Incomplete symlink-containment remediation: an authenticated non-admin user could create a relative symlink via archive extraction, relocate it with WebDAV MOVE, and write through it to arbitrary host paths with the service account's privileges (GHSA-3j46-5w28-f8p2) reporter @Saku0512
|
||||
- [Moderate] Source-root containment bypass: create, write, delete, and move operations now resolve symlinks uncached and enforce the caller's scope, closing escapes through pre-existing out-of-source symlinks on upload, PUT, PATCH, delete, and WebDAV endpoints (GHSA-j764-qg3m-rhv7) reporter @chaoming0625
|
||||
- [Moderate] Source-root containment bypass: create, write, delete, and move operations now resolve symlinks uncached and enforce the caller's scope, closing API escapes through pre-existing out-of-source symlinks on upload, PUT, PATCH, delete, and WebDAV endpoints; RealPathCache entries for affected index paths are invalidated after mutations (GHSA-j764-qg3m-rhv7) reporter @chaoming0625
|
||||
- [Moderate] Preview reads resolve symlinks within the caller's scope so authenticated preview cannot follow an out-of-scope symlink target (GHSA-g2q5-r6ch-v25f) reporter @tao0845
|
||||
- [Moderate] Archive downloads and extraction no longer read or write through symlinks that resolve outside the caller's share or user scope; escaping archive-walk entries are skipped (GHSA-qqf3-2wvq-q4w9) reporter @SmokinCaterpillar
|
||||
- [Moderate] Share directory-archive downloads now apply the share owner's access rules to every entry, closing a bypass where anonymous share visitors could download admin-denied content (GHSA-9fcr-gm58-p839) reporter @Tan-JunWei
|
||||
|
||||
@@ -655,6 +655,7 @@ func DeleteFiles(source, boundIndexPath, indexPath string, isDir bool) error {
|
||||
if utils.CleanIndexPathSegment(indexPath) == "" {
|
||||
return fmt.Errorf("refusing to delete source root directory: %s", indexPath)
|
||||
}
|
||||
deleteIndexPath := indexPath
|
||||
realPath, err := index.GetRealPathForRemoval(boundIndexPath, indexPath)
|
||||
if err != nil {
|
||||
return err
|
||||
@@ -673,9 +674,10 @@ func DeleteFiles(source, boundIndexPath, indexPath string, isDir bool) error {
|
||||
return err
|
||||
}
|
||||
|
||||
// Clear cache entries
|
||||
indexing.RealPathCache.Delete(realPath)
|
||||
indexing.IsDirCache.Delete(realPath + ":isdir")
|
||||
index.InvalidateRealPathCache(deleteIndexPath, boundIndexPath)
|
||||
if parent := utils.GetParentDirectoryPath(deleteIndexPath); parent != "" && parent != "/" {
|
||||
index.InvalidateRealPathCache(parent, boundIndexPath)
|
||||
}
|
||||
|
||||
// Remove metadata from index
|
||||
deleteSuccess := index.DeleteMetadata(indexPath, isDir, isDir)
|
||||
@@ -727,8 +729,7 @@ func RefreshIndex(source string, path string, isDir bool, recursive bool) error
|
||||
if !Exists(realPath) {
|
||||
// Directory no longer exists, remove it from the index
|
||||
// This clears both Directories and DirectoriesLedger maps
|
||||
indexing.RealPathCache.Delete(realPath)
|
||||
indexing.IsDirCache.Delete(realPath + ":isdir")
|
||||
idx.InvalidateRealPathCache(path)
|
||||
idx.DeleteMetadata(path, true, false)
|
||||
return nil
|
||||
}
|
||||
@@ -966,6 +967,11 @@ func WriteDirectory(boundIndexPath string, opts utils.FileOptions) error {
|
||||
}
|
||||
}
|
||||
|
||||
idx.InvalidateRealPathCache(opts.Path, boundIndexPath)
|
||||
if parent := utils.GetParentDirectoryPath(opts.Path); parent != "" && parent != "/" {
|
||||
idx.InvalidateRealPathCache(parent, boundIndexPath)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -1041,6 +1047,11 @@ func writeFile(source, boundIndexPath, path string, in io.Reader) error {
|
||||
}
|
||||
}
|
||||
|
||||
idx.InvalidateRealPathCache(path, boundIndexPath)
|
||||
if parent := utils.GetParentDirectoryPath(path); parent != "" && parent != "/" {
|
||||
idx.InvalidateRealPathCache(parent, boundIndexPath)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,82 @@
|
||||
package web
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
|
||||
"github.com/gtsteffaniak/filebrowser/backend/internal/preview"
|
||||
"github.com/gtsteffaniak/filebrowser/backend/internal/state"
|
||||
)
|
||||
|
||||
func TestResourcePostHandler_RejectsEscapingParentSymlinkMkdir(t *testing.T) {
|
||||
downloadsPath := setupPutHTTPTest(t)
|
||||
outside := filepath.Join(filepath.Dir(downloadsPath), "outside-mkdir")
|
||||
if err := os.MkdirAll(outside, 0755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.Symlink(outside, filepath.Join(downloadsPath, "evil")); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
adminUser, err := state.GetUserByUsername("admin")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
req := httptest.NewRequest(http.MethodPost, "/api/resources?source=Downloads&path=/evil/newdir&isDir=true", nil)
|
||||
rec := httptest.NewRecorder()
|
||||
|
||||
status, handlerErr := ResourcePostHandler(rec, req, &requestContext{User: &adminUser})
|
||||
if status != http.StatusForbidden {
|
||||
t.Fatalf("status=%d err=%v", status, handlerErr)
|
||||
}
|
||||
if handlerErr == nil {
|
||||
t.Fatal("expected mkdir to be forbidden")
|
||||
}
|
||||
if _, err := os.Stat(filepath.Join(outside, "newdir")); !os.IsNotExist(err) {
|
||||
t.Fatal("mkdir escaped outside the source via symlink parent")
|
||||
}
|
||||
}
|
||||
|
||||
func TestResourceDeleteHandler_RejectsEscapingIntermediate(t *testing.T) {
|
||||
downloadsPath := setupPutHTTPTest(t)
|
||||
if preview.GetService() == nil {
|
||||
if err := preview.StartPreviewGenerator(1, t.TempDir()); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
outsideDir := filepath.Join(filepath.Dir(downloadsPath), "outside-delete")
|
||||
if err := os.MkdirAll(outsideDir, 0755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
victim := filepath.Join(outsideDir, "victim.txt")
|
||||
if err := os.WriteFile(victim, []byte("keep"), 0644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.Symlink(outsideDir, filepath.Join(downloadsPath, "evil")); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
adminUser, err := state.GetUserByUsername("admin")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
req := httptest.NewRequest(http.MethodDelete, "/api/resources?source=Downloads&path=/evil/victim.txt", nil)
|
||||
rec := httptest.NewRecorder()
|
||||
|
||||
status, handlerErr := resourceDeleteHandler(rec, req, &requestContext{User: &adminUser})
|
||||
if status != http.StatusForbidden {
|
||||
t.Fatalf("status=%d err=%v", status, handlerErr)
|
||||
}
|
||||
if handlerErr == nil {
|
||||
t.Fatal("expected delete to be forbidden")
|
||||
}
|
||||
got, err := os.ReadFile(victim)
|
||||
if err != nil || string(got) != "keep" {
|
||||
t.Fatalf("delete escaped: err=%v content=%q", err, got)
|
||||
}
|
||||
}
|
||||
@@ -1241,6 +1241,61 @@ func (idx *Index) getRealPathInternal(boundIndexPath string, enforceScope bool,
|
||||
return realPath, isDir, nil
|
||||
}
|
||||
|
||||
// realPathCacheJoinedPath builds the filesystem join used in RealPathCache keys.
|
||||
func realPathCacheJoinedPath(sourceRoot, indexPath string) string {
|
||||
seg := utils.CleanIndexPathSegment(indexPath)
|
||||
if seg == "" {
|
||||
return sourceRoot
|
||||
}
|
||||
return filepath.Join(sourceRoot, seg)
|
||||
}
|
||||
|
||||
func realPathCacheIndexAncestors(indexPath string) []string {
|
||||
rel := utils.CleanIndexPathSegment(indexPath)
|
||||
if rel == "" {
|
||||
return []string{"/"}
|
||||
}
|
||||
parts := strings.Split(rel, "/")
|
||||
ancestors := []string{"/"}
|
||||
for i := range parts {
|
||||
ancestors = append(ancestors, "/"+strings.Join(parts[:i+1], "/"))
|
||||
}
|
||||
return ancestors
|
||||
}
|
||||
|
||||
// InvalidateRealPathCache drops cached symlink resolutions for indexPath and its
|
||||
// ancestors. scopeBounds should include any caller scope prefixes used with
|
||||
// GetRealPathScoped (e.g. user or share path).
|
||||
func (idx *Index) InvalidateRealPathCache(indexPath string, scopeBounds ...string) {
|
||||
boundSet := map[string]struct{}{"/": {}, "": {}}
|
||||
for _, b := range scopeBounds {
|
||||
b = strings.TrimRight(strings.TrimSpace(b), "/")
|
||||
if b == "" || b == "/" {
|
||||
boundSet["/"] = struct{}{}
|
||||
boundSet[""] = struct{}{}
|
||||
continue
|
||||
}
|
||||
if !strings.HasPrefix(b, "/") {
|
||||
b = "/" + b
|
||||
}
|
||||
boundSet[b] = struct{}{}
|
||||
}
|
||||
bounds := make([]string, 0, len(boundSet))
|
||||
for b := range boundSet {
|
||||
bounds = append(bounds, b)
|
||||
}
|
||||
for _, ip := range realPathCacheIndexAncestors(indexPath) {
|
||||
joined := realPathCacheJoinedPath(idx.Path, ip)
|
||||
for _, enforceScope := range []bool{false, true} {
|
||||
for _, bound := range bounds {
|
||||
key := fmt.Sprintf("realpath|scoped=%t|bound=%q|path=%q", enforceScope, bound, joined)
|
||||
RealPathCache.Delete(key)
|
||||
IsDirCache.Delete(key + ":isdir")
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// GetRealPathForMutation resolves an index path for a mutating filesystem
|
||||
// operation (create/overwrite/mkdir/move-into). When the leaf exists it is fully
|
||||
// resolved; otherwise the deepest existing ancestor is resolved and the missing
|
||||
|
||||
@@ -2,6 +2,7 @@ package indexing
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
@@ -163,6 +164,30 @@ func TestGetRealPathScoped_CacheKeyDoesNotCollideWithUnscoped(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestInvalidateRealPathCache_ClearsGetRealPathEntry(t *testing.T) {
|
||||
sourceRoot, userScope := setupSymlinkEscapeFixture(t)
|
||||
idx := symlinkTestIndex(sourceRoot)
|
||||
|
||||
userDir := filepath.Join(sourceRoot, "user1")
|
||||
if err := os.WriteFile(filepath.Join(userDir, "foo"), []byte("x"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
if _, _, err := idx.GetRealPath("/user1/foo"); err != nil {
|
||||
t.Fatalf("warm cache: %v", err)
|
||||
}
|
||||
joined := realPathCacheJoinedPath(sourceRoot, "/user1/foo")
|
||||
key := fmt.Sprintf("realpath|scoped=%t|bound=%q|path=%q", false, "/", joined)
|
||||
if _, ok := RealPathCache.Get(key); !ok {
|
||||
t.Fatalf("expected cache key %q", key)
|
||||
}
|
||||
|
||||
idx.InvalidateRealPathCache("/user1/foo", userScope)
|
||||
if _, ok := RealPathCache.Get(key); ok {
|
||||
t.Fatal("InvalidateRealPathCache should drop unscoped cache entry")
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetRealPathForMutation_ExistingInScope(t *testing.T) {
|
||||
sourceRoot, userScope := setupSymlinkEscapeFixture(t)
|
||||
idx := symlinkTestIndex(sourceRoot)
|
||||
|
||||
Reference in new issue
Block a user