From 9bd79c3aaeb4a55b0e69cf8976c4a258db2f5e06 Mon Sep 17 00:00:00 2001 From: Henrique Dias Date: Sat, 25 Jul 2026 07:58:43 +0200 Subject: [PATCH] fix(http): delete abandoned TUS uploads through the scoped filesystem The in-memory upload cache deleted expired incomplete uploads with a raw os.Remove on the absolute path it had stored, bypassing ScopedFs entirely. Because the stored path is only lexically cleaned (no symlink evaluation) and os.Remove resolves symlinked parent directories, a Create-only user could register an upload and then, within the 3-minute TTL, swap an in-scope ancestor directory for a symlink so the eviction deleted an arbitrary file outside their scope. Carry a removal callback with each cache entry and invoke it on eviction instead of os.Remove. For TUS uploads the callback deletes via the uploading user's scoped filesystem, whose Remove is guarded by the same within() check that CVE-2026-55667 added, so eviction can no longer follow a symlink out of scope. The redis backend ignores the callback (it never deleted partial files). Refs GHSA-m9f5-2232-frp6 --- http/tus_handlers.go | 9 +++++++-- http/upload_cache_memory.go | 36 ++++++++++++++++++++++++++---------- http/upload_cache_redis.go | 4 +++- 3 files changed, 36 insertions(+), 13 deletions(-) diff --git a/http/tus_handlers.go b/http/tus_handlers.go index fe7ae280..db2e6bc0 100644 --- a/http/tus_handlers.go +++ b/http/tus_handlers.go @@ -109,8 +109,13 @@ func tusPostHandler(cache UploadCache) handleFunc { return http.StatusBadRequest, fmt.Errorf("invalid upload length: %w", err) } - // Enables the user to utilize the PATCH endpoint for uploading file data - cache.Register(file.RealPath(), uploadLength) + // Enables the user to utilize the PATCH endpoint for uploading file data. + // The removal callback deletes an abandoned upload through the user's + // scoped filesystem, so eviction cannot follow a symlink out of scope. + uploadPath := r.URL.Path + cache.Register(file.RealPath(), uploadLength, func() error { + return d.user.Fs.Remove(uploadPath) + }) basePath := "/" + strings.Trim(strings.TrimSpace(d.server.BaseURL), "/") if basePath == "/" { diff --git a/http/upload_cache_memory.go b/http/upload_cache_memory.go index 5b080fec..8bb8cc89 100644 --- a/http/upload_cache_memory.go +++ b/http/upload_cache_memory.go @@ -3,7 +3,6 @@ package fbhttp import ( "context" "fmt" - "os" "time" "github.com/jellydator/ttlcache/v3" @@ -15,8 +14,11 @@ const uploadCacheTTL = 3 * time.Minute // Allows for different backends (e.g. in-memory or redis) // to support both single instance and multi replica deployments. type UploadCache interface { - // Register stores an upload with its expected file size - Register(filePath string, fileSize int64) + // Register stores an upload with its expected file size. remove is called if + // the upload expires before completion, to delete the partial file; it must + // route through the uploading user's scoped filesystem so that eviction + // cannot follow a symlink out of the user's scope. + Register(filePath string, fileSize int64, remove func() error) // Complete removes an upload from the cache Complete(filePath string) @@ -31,17 +33,31 @@ type UploadCache interface { Close() } +// memoryUploadEntry is the value stored for each active upload. +type memoryUploadEntry struct { + size int64 + remove func() error +} + // memoryUploadCache is an upload cache for single replica deployments type memoryUploadCache struct { - cache *ttlcache.Cache[string, int64] + cache *ttlcache.Cache[string, memoryUploadEntry] } func newMemoryUploadCache() *memoryUploadCache { - cache := ttlcache.New[string, int64]() - cache.OnEviction(func(_ context.Context, reason ttlcache.EvictionReason, item *ttlcache.Item[string, int64]) { + cache := ttlcache.New[string, memoryUploadEntry]() + cache.OnEviction(func(_ context.Context, reason ttlcache.EvictionReason, item *ttlcache.Item[string, memoryUploadEntry]) { if reason == ttlcache.EvictionReasonExpired { fmt.Printf("deleting incomplete upload file: \"%s\"\n", item.Key()) - os.Remove(item.Key()) + // Delete through the scoped removal callback rather than a raw + // os.Remove on the cached path, so an ancestor directory swapped for + // a symlink during the TTL window cannot redirect the delete outside + // the user's scope. + if remove := item.Value().remove; remove != nil { + if err := remove(); err != nil { + fmt.Printf("failed to delete incomplete upload file %q: %v\n", item.Key(), err) + } + } } }) go cache.Start() @@ -49,8 +65,8 @@ func newMemoryUploadCache() *memoryUploadCache { return &memoryUploadCache{cache: cache} } -func (c *memoryUploadCache) Register(filePath string, fileSize int64) { - c.cache.Set(filePath, fileSize, uploadCacheTTL) +func (c *memoryUploadCache) Register(filePath string, fileSize int64, remove func() error) { + c.cache.Set(filePath, memoryUploadEntry{size: fileSize, remove: remove}, uploadCacheTTL) } func (c *memoryUploadCache) Complete(filePath string) { @@ -62,7 +78,7 @@ func (c *memoryUploadCache) GetLength(filePath string) (int64, error) { if item == nil { return 0, fmt.Errorf("no active upload found for the given path") } - return item.Value(), nil + return item.Value().size, nil } func (c *memoryUploadCache) Touch(filePath string) { diff --git a/http/upload_cache_redis.go b/http/upload_cache_redis.go index 9f2025bc..ff3d5123 100644 --- a/http/upload_cache_redis.go +++ b/http/upload_cache_redis.go @@ -39,7 +39,9 @@ func (c *redisUploadCache) filePathKey(filePath string) string { return "filebrowser:upload:" + filePath } -func (c *redisUploadCache) Register(filePath string, fileSize int64) { +// Register stores the upload length. The scoped removal callback is unused by +// the redis backend, which does not delete partial files on eviction. +func (c *redisUploadCache) Register(filePath string, fileSize int64, _ func() error) { err := c.client.Set(context.Background(), c.filePathKey(filePath), fileSize, uploadCacheTTL).Err() if err != nil { log.Printf("failed to register upload in redis cache: %v", err)