mirror of
https://github.com/filebrowser/filebrowser.git
synced 2026-07-17 16:36:49 +00:00
fix: recursive check
This commit is contained in:
parent
67ed670d92
commit
3406d3d7f9
6 changed files with 94 additions and 13 deletions
|
|
@ -475,10 +475,6 @@ func (i *FileInfo) readListing(checker rules.Checker, readHeader bool, calcImgRe
|
|||
isSymlink, isInvalidLink := false, false
|
||||
if IsSymlink(f.Mode()) {
|
||||
isSymlink = true
|
||||
// A symlink whose on-disk target escapes the scoped root must not be
|
||||
// followed, otherwise the listing would leak the target's metadata
|
||||
// (and downstream access) for files outside the user's scope or the
|
||||
// shared subtree.
|
||||
if ok, scopeErr := WithinScope(i.Fs, fPath); scopeErr != nil || !ok {
|
||||
continue
|
||||
}
|
||||
|
|
|
|||
74
fileutils/copy_test.go
Normal file
74
fileutils/copy_test.go
Normal file
|
|
@ -0,0 +1,74 @@
|
|||
package fileutils
|
||||
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
|
||||
"github.com/spf13/afero"
|
||||
)
|
||||
|
||||
// Copying an in-scope directory that contains a symlink whose target escapes
|
||||
// the user's scope must not dereference that symlink into the destination.
|
||||
// Otherwise a scoped user could exfiltrate out-of-scope file content via the
|
||||
// recursive copy path (GHSA-c2gv-wf5f-hjhh, an incomplete fix of
|
||||
// GHSA-239w-m3h6-ch8v).
|
||||
func TestCopyDoesNotDereferenceEscapingSymlink(t *testing.T) {
|
||||
base := t.TempDir()
|
||||
scope := filepath.Join(base, "scope")
|
||||
if err := os.MkdirAll(filepath.Join(scope, "srcdir"), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
// A secret living outside the scope.
|
||||
secret := filepath.Join(base, "secret.txt")
|
||||
if err := os.WriteFile(secret, []byte("OUT-OF-SCOPE-SECRET"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
// An escaping symlink planted inside the user's scope.
|
||||
if err := os.Symlink(secret, filepath.Join(scope, "srcdir", "link.txt")); err != nil {
|
||||
t.Skipf("cannot create symlink: %v", err)
|
||||
}
|
||||
|
||||
afs := afero.NewBasePathFs(afero.NewOsFs(), scope)
|
||||
|
||||
err := Copy(afs, "/srcdir", "/dstdir", 0o644, 0o755)
|
||||
if err == nil {
|
||||
t.Fatal("expected copy of a directory containing an escaping symlink to fail")
|
||||
}
|
||||
|
||||
// The escaping symlink's target content must not have landed in scope.
|
||||
if data, readErr := afero.ReadFile(afs, "/dstdir/link.txt"); readErr == nil {
|
||||
t.Fatalf("escaping symlink was dereferenced into scope: got %q", string(data))
|
||||
}
|
||||
}
|
||||
|
||||
// A symlink whose target stays within scope is legitimate and must still be
|
||||
// copied (dereferenced) so the fix does not over-block normal usage.
|
||||
func TestCopyAllowsInScopeSymlink(t *testing.T) {
|
||||
scope := t.TempDir()
|
||||
if err := os.MkdirAll(filepath.Join(scope, "srcdir", "real"), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(scope, "srcdir", "real", "f.txt"), []byte("in-scope"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.Symlink(filepath.Join(scope, "srcdir", "real", "f.txt"), filepath.Join(scope, "srcdir", "link.txt")); err != nil {
|
||||
t.Skipf("cannot create symlink: %v", err)
|
||||
}
|
||||
|
||||
afs := afero.NewBasePathFs(afero.NewOsFs(), scope)
|
||||
|
||||
if err := Copy(afs, "/srcdir", "/dstdir", 0o644, 0o755); err != nil {
|
||||
t.Fatalf("expected copy of an in-scope symlink to succeed, got: %v", err)
|
||||
}
|
||||
|
||||
data, err := afero.ReadFile(afs, "/dstdir/link.txt")
|
||||
if err != nil {
|
||||
t.Fatalf("expected in-scope symlink to be copied, got: %v", err)
|
||||
}
|
||||
if string(data) != "in-scope" {
|
||||
t.Fatalf("unexpected copied content: %q", string(data))
|
||||
}
|
||||
}
|
||||
|
|
@ -3,14 +3,24 @@ package fileutils
|
|||
import (
|
||||
"errors"
|
||||
"io/fs"
|
||||
"os"
|
||||
|
||||
"github.com/spf13/afero"
|
||||
|
||||
"github.com/filebrowser/filebrowser/v2/files"
|
||||
)
|
||||
|
||||
// CopyDir copies a directory from source to dest and all
|
||||
// of its sub-directories. It doesn't stop if it finds an error
|
||||
// during the copy. Returns an error if any.
|
||||
func CopyDir(afs afero.Fs, source, dest string, fileMode, dirMode fs.FileMode) error {
|
||||
if ok, err := files.WithinScope(afs, source); err != nil || !ok {
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
return os.ErrPermission
|
||||
}
|
||||
|
||||
// Get properties of source.
|
||||
srcinfo, err := afs.Stat(source)
|
||||
if err != nil {
|
||||
|
|
|
|||
|
|
@ -8,6 +8,8 @@ import (
|
|||
"path/filepath"
|
||||
|
||||
"github.com/spf13/afero"
|
||||
|
||||
"github.com/filebrowser/filebrowser/v2/files"
|
||||
)
|
||||
|
||||
// MoveFile moves file from src to dst.
|
||||
|
|
@ -32,6 +34,13 @@ func MoveFile(afs afero.Fs, src, dst string, fileMode, dirMode fs.FileMode) erro
|
|||
// CopyFile copies a file from source to dest and returns
|
||||
// an error if any.
|
||||
func CopyFile(afs afero.Fs, source, dest string, fileMode, dirMode fs.FileMode) error {
|
||||
if ok, err := files.WithinScope(afs, source); err != nil || !ok {
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
return os.ErrPermission
|
||||
}
|
||||
|
||||
// Open the source file.
|
||||
src, err := afs.Open(source)
|
||||
if err != nil {
|
||||
|
|
|
|||
|
|
@ -228,10 +228,6 @@ func resourcePatchHandler(fileCache FileCache) handleFunc {
|
|||
return http.StatusForbidden, nil
|
||||
}
|
||||
|
||||
// Refuse to copy/move through a symlink that escapes the user's scope,
|
||||
// for either the source (read escape) or the destination (write
|
||||
// escape). fileutils.Copy/MoveFile operate on the raw afero FS and
|
||||
// follow symlinks, so they bypass the guards in stat()/writeFile().
|
||||
for _, p := range []string{src, dst} {
|
||||
if ok, scopeErr := files.WithinScope(d.user.Fs, p); scopeErr != nil || !ok {
|
||||
if scopeErr != nil {
|
||||
|
|
@ -308,9 +304,6 @@ func addVersionSuffix(source string, afs afero.Fs) string {
|
|||
}
|
||||
|
||||
func writeFile(afs afero.Fs, dst string, in io.Reader, fileMode, dirMode fs.FileMode) (os.FileInfo, error) {
|
||||
// Refuse to write through a symlink that escapes the user's scope, so an
|
||||
// overwrite of an existing escaping symlink cannot modify a file outside
|
||||
// the boundary.
|
||||
if ok, err := files.WithinScope(afs, dst); err != nil || !ok {
|
||||
return nil, os.ErrPermission
|
||||
}
|
||||
|
|
|
|||
|
|
@ -166,8 +166,7 @@ func tusPatchHandler(cache UploadCache) handleFunc {
|
|||
if r.Header.Get("Content-Type") != "application/offset+octet-stream" {
|
||||
return http.StatusUnsupportedMediaType, nil
|
||||
}
|
||||
// Defense in depth: refuse to write through a symlink that escapes the
|
||||
// scope, in case the target path is (or sits behind) an escaping link.
|
||||
|
||||
if ok, scopeErr := files.WithinScope(d.user.Fs, r.URL.Path); scopeErr != nil || !ok {
|
||||
return http.StatusForbidden, nil
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue