diff --git a/weed/command/filer_sync_process_test.go b/weed/command/filer_sync_process_test.go index 132902134a..3e81e0d914 100644 --- a/weed/command/filer_sync_process_test.go +++ b/weed/command/filer_sync_process_test.go @@ -1,11 +1,14 @@ package command import ( + "os" + "path/filepath" "strings" "testing" "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" "github.com/seaweedfs/seaweedfs/weed/replication/sink" + "github.com/seaweedfs/seaweedfs/weed/replication/sink/localsink" "github.com/seaweedfs/seaweedfs/weed/replication/source" "github.com/seaweedfs/seaweedfs/weed/util" ) @@ -343,3 +346,35 @@ func TestGenProcessFunctionRenameFromExcludedDirBecomesCreate(t *testing.T) { t.Fatalf("unexpected delete/update calls: deletes=%v updates=%v", dataSink.deleteKeys, dataSink.updateKeys) } } + +func TestGenProcessFunctionLocalSinkStaysInsideTarget(t *testing.T) { + root := t.TempDir() + target := filepath.Join(root, "backup") + dataSink := &localsink.LocalSink{Dir: target} + dataSink.SetSourceFiler(&source.FilerSource{}) + processFn := genProcessFunction("/", target, nil, nil, nil, nil, dataSink, true, false) + + for _, dir := range []string{"/safe", "/../escape"} { + err := processFn(&filer_pb.SubscribeMetadataResponse{ + Directory: dir, + EventNotification: &filer_pb.EventNotification{ + NewParentPath: dir, + NewEntry: &filer_pb.Entry{ + Name: "marker.txt", + Content: []byte("marker"), + Attributes: &filer_pb.FuseAttributes{FileMode: 0644, FileSize: 6}, + }, + }, + }) + if err != nil { + t.Fatalf("processFn %s: %v", dir, err) + } + } + + if _, err := os.Stat(filepath.Join(target, "safe", "marker.txt")); err != nil { + t.Fatalf("inside file missing: %v", err) + } + if _, err := os.Stat(filepath.Join(root, "escape")); !os.IsNotExist(err) { + t.Fatalf("local sink wrote outside %s: %v", target, err) + } +} diff --git a/weed/filer/filer.go b/weed/filer/filer.go index bce66f5889..bae29d10bf 100644 --- a/weed/filer/filer.go +++ b/weed/filer/filer.go @@ -501,6 +501,10 @@ func (f *Filer) CreateEntry(ctx context.Context, entry *Entry, existing *Entry, return filer_pb.ErrEntryNameTooLong } + if entry.FullPath.HasDotSegment() { + return fmt.Errorf("invalid path %s", entry.FullPath) + } + if entry.IsDirectory() { entry.Attr.TtlSec = 0 } @@ -794,6 +798,9 @@ func (f *Filer) EnsureDirectoryEntry(ctx context.Context, dirPath util.FullPath, } func (f *Filer) UpdateEntry(ctx context.Context, oldEntry, entry *Entry, isFromOtherCluster bool) (err error) { + if entry.FullPath.HasDotSegment() { + return fmt.Errorf("invalid path %s", entry.FullPath) + } ctx = f.syncWriteContext(ctx, entry.FullPath) if oldEntry != nil { entry.Attr.Crtime = oldEntry.Attr.Crtime diff --git a/weed/replication/sink/localsink/local_sink.go b/weed/replication/sink/localsink/local_sink.go index 8debf7993a..b66e8abff4 100644 --- a/weed/replication/sink/localsink/local_sink.go +++ b/weed/replication/sink/localsink/local_sink.go @@ -3,6 +3,7 @@ package localsink import ( "context" "fmt" + "math/rand/v2" "os" "path/filepath" "strings" @@ -39,6 +40,34 @@ func (localsink *LocalSink) isMultiPartEntry(key string) bool { return strings.HasSuffix(key, ".part") && strings.Contains(key, "/"+s3_constants.MultipartUploadsFolder+"/") } +func (localsink *LocalSink) isInsideDir(key string) (string, bool) { + rel, err := filepath.Rel(filepath.Clean(localsink.Dir), filepath.Clean(key)) + if err == nil && filepath.IsLocal(rel) { + return rel, true + } + glog.Warningf("local sink: skip %s outside %s", key, localsink.Dir) + return "", false +} + +// resolveInsideDir resolves symlinks in the parent of rel while it stays inside the sink directory. +func (localsink *LocalSink) resolveInsideDir(rel string) string { + base, err := filepath.EvalSymlinks(localsink.Dir) + if err != nil { + return rel + } + dir, rest := filepath.Dir(rel), filepath.Base(rel) + for dir != "." { + if resolved, err := filepath.EvalSymlinks(filepath.Join(base, dir)); err == nil { + if inside, err := filepath.Rel(base, resolved); err == nil && filepath.IsLocal(inside) { + return filepath.Join(inside, rest) + } + return rel + } + dir, rest = filepath.Dir(dir), filepath.Join(filepath.Base(dir), rest) + } + return rel +} + func (localsink *LocalSink) initialize(dir string, isIncremental bool) error { localsink.Dir = dir localsink.isIncremental = isIncremental @@ -69,8 +98,17 @@ func (localsink *LocalSink) DeleteEntry(key string, isDirectory, deleteIncludeCh if localsink.isMultiPartEntry(key) { return nil } + rel, ok := localsink.isInsideDir(key) + if !ok { + return nil + } glog.V(4).Infof("Delete Entry key: %s", key) - if err := os.Remove(key); err != nil { + root, err := os.OpenRoot(localsink.Dir) + if err == nil { + err = root.Remove(localsink.resolveInsideDir(rel)) + root.Close() + } + if err != nil { glog.V(0).Infof("remove entry key %s: %s", key, err) } return nil @@ -81,6 +119,10 @@ func (localsink *LocalSink) CreateEntry(key string, entry *filer_pb.Entry, signa if entry.IsDirectory || localsink.isMultiPartEntry(key) { return nil } + rel, ok := localsink.isInsideDir(key) + if !ok { + return nil + } glog.V(4).Infof("Create Entry key: %s", key) totalSize := filer.FileSize(entry) @@ -89,35 +131,37 @@ func (localsink *LocalSink) CreateEntry(key string, entry *filer_pb.Entry, signa return fmt.Errorf("resolve chunks of %s: %w", key, err) } - dir := filepath.Dir(key) - - if _, err := os.Stat(dir); os.IsNotExist(err) { - glog.V(4).Infof("Create Directory key: %s", dir) - if err = os.MkdirAll(dir, 0755); err != nil { - return err - } + if err := os.MkdirAll(localsink.Dir, 0755); err != nil { + return err } + root, err := os.OpenRoot(localsink.Dir) + if err != nil { + return err + } + defer root.Close() - if entry.IsDirectory { - return os.Mkdir(key, os.FileMode(entry.Attributes.FileMode)) + rel = localsink.resolveInsideDir(rel) + dir := filepath.Dir(rel) + if err := root.MkdirAll(dir, 0755); err != nil { + return err } mode := os.FileMode(entry.Attributes.FileMode) - shortFileName := util.ToShortFileName(key) + shortFileName := util.ToShortFileName(rel) // Write to a temp file in the same directory, then atomically rename // on success. This prevents leaving a truncated/empty file if // decryption or chunk copy fails. - tmpFile, err := os.CreateTemp(dir, ".seaweedfs-tmp-*") + tmpName := filepath.Join(dir, fmt.Sprintf(".seaweedfs-tmp-%d", rand.Uint64())) + tmpFile, err := root.OpenFile(tmpName, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0600) if err != nil { return err } - tmpName := tmpFile.Name() defer func() { // Clean up temp file on any error (rename removes it on success) if tmpFile != nil { tmpFile.Close() - os.Remove(tmpName) + root.Remove(tmpName) } }() @@ -151,8 +195,8 @@ func (localsink *LocalSink) CreateEntry(key string, entry *filer_pb.Entry, signa tmpFile = nil // prevent deferred cleanup // Atomic rename into final destination - if err := os.Rename(tmpName, shortFileName); err != nil { - os.Remove(tmpName) + if err := root.Rename(tmpName, shortFileName); err != nil { + root.Remove(tmpName) return err } @@ -164,6 +208,9 @@ func (localsink *LocalSink) UpdateEntry(key string, oldEntry *filer_pb.Entry, ne if localsink.isMultiPartEntry(key) { return true, nil } + if _, ok := localsink.isInsideDir(key); !ok { + return true, nil + } glog.V(4).Infof("Update Entry key: %s", key) // do delete and create foundExistingEntry = util.FileExists(key) diff --git a/weed/replication/sink/localsink/local_sink_dir_test.go b/weed/replication/sink/localsink/local_sink_dir_test.go new file mode 100644 index 0000000000..c044ca500a --- /dev/null +++ b/weed/replication/sink/localsink/local_sink_dir_test.go @@ -0,0 +1,146 @@ +package localsink + +import ( + "os" + "path/filepath" + "testing" + + "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" + "github.com/seaweedfs/seaweedfs/weed/replication/source" +) + +func TestLocalSinkStaysInsideDir(t *testing.T) { + root := t.TempDir() + dir := filepath.Join(root, "backup") + sink := &LocalSink{} + sink.initialize(dir, false) + sink.SetSourceFiler(&source.FilerSource{}) + + entry := func() *filer_pb.Entry { + return &filer_pb.Entry{ + Name: "marker.txt", + Content: []byte("marker"), + Attributes: &filer_pb.FuseAttributes{FileMode: 0644, FileSize: 6}, + } + } + + inside := filepath.Join(dir, "safe", "marker.txt") + if err := sink.CreateEntry(inside, entry(), nil); err != nil { + t.Fatalf("create inside: %v", err) + } + if _, err := os.Stat(inside); err != nil { + t.Fatalf("inside file missing: %v", err) + } + + outside := filepath.Join(dir, "..", "escape", "marker.txt") + if err := sink.CreateEntry(outside, entry(), nil); err != nil { + t.Fatalf("create outside: %v", err) + } + if _, err := sink.UpdateEntry(outside, nil, "", entry(), false, nil); err != nil { + t.Fatalf("update outside: %v", err) + } + if _, err := os.Stat(filepath.Join(root, "escape")); !os.IsNotExist(err) { + t.Fatalf("sink wrote outside %s: %v", dir, err) + } + + victim := filepath.Join(root, "victim.txt") + if err := os.WriteFile(victim, []byte("keep"), 0644); err != nil { + t.Fatal(err) + } + if err := sink.DeleteEntry(filepath.Join(dir, "..", "victim.txt"), false, false, nil); err != nil { + t.Fatalf("delete outside: %v", err) + } + if _, err := os.Stat(victim); err != nil { + t.Fatalf("sink deleted a file outside %s: %v", dir, err) + } +} + +func TestLocalSinkDoesNotFollowSymlinksOutOfDir(t *testing.T) { + root := t.TempDir() + dir := filepath.Join(root, "backup") + outside := filepath.Join(root, "outside") + for _, d := range []string{dir, outside} { + if err := os.MkdirAll(d, 0755); err != nil { + t.Fatal(err) + } + } + if err := os.Symlink(outside, filepath.Join(dir, "link")); err != nil { + t.Skipf("symlink: %v", err) + } + sink := &LocalSink{} + sink.initialize(dir, false) + sink.SetSourceFiler(&source.FilerSource{}) + + entry := &filer_pb.Entry{ + Name: "marker.txt", + Content: []byte("marker"), + Attributes: &filer_pb.FuseAttributes{FileMode: 0644, FileSize: 6}, + } + if err := sink.CreateEntry(filepath.Join(dir, "link", "marker.txt"), entry, nil); err == nil { + t.Fatalf("create through a symlink out of %s succeeded", dir) + } + if _, err := os.Stat(filepath.Join(outside, "marker.txt")); !os.IsNotExist(err) { + t.Fatalf("sink wrote through a symlink out of %s: %v", dir, err) + } + + victim := filepath.Join(outside, "victim.txt") + if err := os.WriteFile(victim, []byte("keep"), 0644); err != nil { + t.Fatal(err) + } + if err := sink.DeleteEntry(filepath.Join(dir, "link", "victim.txt"), false, false, nil); err != nil { + t.Fatalf("delete through link: %v", err) + } + if _, err := os.Stat(victim); err != nil { + t.Fatalf("sink deleted through a symlink out of %s: %v", dir, err) + } + + if err := os.Mkdir(filepath.Join(dir, "real"), 0755); err != nil { + t.Fatal(err) + } + if err := os.Symlink("real", filepath.Join(dir, "inner")); err != nil { + t.Fatal(err) + } + if err := sink.CreateEntry(filepath.Join(dir, "inner", "marker.txt"), entry, nil); err != nil { + t.Fatalf("create through a symlink inside %s: %v", dir, err) + } + if _, err := os.Stat(filepath.Join(dir, "real", "marker.txt")); err != nil { + t.Fatalf("inside symlink target missing: %v", err) + } +} + +func TestLocalSinkFollowsAbsoluteSymlinksInsideDir(t *testing.T) { + dir := filepath.Join(t.TempDir(), "backup") + target := filepath.Join(dir, "real") + if err := os.MkdirAll(target, 0755); err != nil { + t.Fatal(err) + } + if err := os.Symlink(target, filepath.Join(dir, "abs")); err != nil { + t.Skipf("symlink: %v", err) + } + sink := &LocalSink{} + sink.initialize(dir, false) + sink.SetSourceFiler(&source.FilerSource{}) + + entry := &filer_pb.Entry{ + Name: "marker.txt", + Content: []byte("marker"), + Attributes: &filer_pb.FuseAttributes{FileMode: 0644, FileSize: 6}, + } + for _, name := range []string{"marker.txt", filepath.Join("sub", "marker.txt")} { + if err := sink.CreateEntry(filepath.Join(dir, "abs", name), entry, nil); err != nil { + t.Fatalf("create %s through an absolute symlink inside %s: %v", name, dir, err) + } + if _, err := os.Stat(filepath.Join(target, name)); err != nil { + t.Fatalf("absolute symlink target %s missing: %v", name, err) + } + if err := sink.DeleteEntry(filepath.Join(dir, "abs", name), false, false, nil); err != nil { + t.Fatalf("delete %s through an absolute symlink: %v", name, err) + } + if _, err := os.Stat(filepath.Join(target, name)); !os.IsNotExist(err) { + t.Fatalf("delete through an absolute symlink left %s: %v", name, err) + } + } + if _, err := os.Lstat(filepath.Join(dir, "abs")); err != nil { + t.Fatalf("symlink itself removed: %v", err) + } +} diff --git a/weed/server/filer_grpc_server_create_route_test.go b/weed/server/filer_grpc_server_create_route_test.go index 0c266c5397..8bb4bbbc33 100644 --- a/weed/server/filer_grpc_server_create_route_test.go +++ b/weed/server/filer_grpc_server_create_route_test.go @@ -232,3 +232,40 @@ func TestCreateEntryFailsWhenPriorOwnerIsUnreachable(t *testing.T) { t.Fatal("a create routed away must not also write locally") } } + +func TestCreateEntryRejectsDotSegments(t *testing.T) { + fs, store := createRouteServer(t) + + for _, tc := range []struct{ dir, name string }{ + {"/../escape", "marker.txt"}, + {"/test/./sub", "marker.txt"}, + {"/test", ".."}, + } { + req := createReq(tc.name, false) + req.Directory = tc.dir + resp, err := fs.CreateEntry(context.Background(), req) + if err == nil && resp.Error == "" { + t.Fatalf("CreateEntry %s/%s must be rejected", tc.dir, tc.name) + } + if _, found := store.entries[string(util.NewFullPath(tc.dir, tc.name))]; found { + t.Fatalf("CreateEntry %s/%s stored the raw path", tc.dir, tc.name) + } + } + + resp, err := fs.CreateEntry(context.Background(), createReq("marker.txt", false)) + if err != nil || resp.Error != "" { + t.Fatalf("CreateEntry /test/marker.txt: err=%v resp=%v", err, resp) + } + + update := &filer_pb.UpdateEntryRequest{ + Directory: "/x/../test", + Entry: createReq("marker.txt", false).Entry, + } + update.Entry.Attributes.Mtime++ + if _, err := fs.UpdateEntry(context.Background(), update); err == nil { + t.Fatal("UpdateEntry with a dot segment must be rejected") + } + if _, found := store.entries["/x/../test/marker.txt"]; found { + t.Fatal("UpdateEntry stored the raw path") + } +} diff --git a/weed/server/filer_grpc_server_rename_test.go b/weed/server/filer_grpc_server_rename_test.go index 10d6941406..b793025472 100644 --- a/weed/server/filer_grpc_server_rename_test.go +++ b/weed/server/filer_grpc_server_rename_test.go @@ -559,3 +559,32 @@ func TestAtomicRenameReleasesQueuedStampsOnCommitFailure(t *testing.T) { t.Fatalf("LocalDeliveredThroughTsNs = %d, want %d: a leaked stamp still caps the claim", got, now) } } + +func TestAtomicRenameEntryRejectsDotSegmentTarget(t *testing.T) { + for _, tc := range []struct{ dir, name string }{ + {"/../escape", "dst.txt"}, + {"/", ".."}, + } { + store := newRenameTestStore() + store.entries["/src.txt"] = newFileEntry("/src.txt", 101) + f := newRenameTestFiler(t, store) + f.DirBucketsPath = "/buckets" + server := &FilerServer{filer: f, entryLockTable: util.NewLockTable[util.FullPath]()} + + _, err := server.AtomicRenameEntry(context.Background(), &filer_pb.AtomicRenameEntryRequest{ + OldDirectory: "/", + OldName: "src.txt", + NewDirectory: tc.dir, + NewName: tc.name, + }) + if err == nil { + t.Fatalf("rename to %s/%s must be rejected", tc.dir, tc.name) + } + if _, found := store.entries["/src.txt"]; !found { + t.Fatalf("rename to %s/%s removed the source", tc.dir, tc.name) + } + if len(store.entries) != 1 { + t.Fatalf("rename to %s/%s left entries %v", tc.dir, tc.name, store.entries) + } + } +} diff --git a/weed/util/fullpath.go b/weed/util/fullpath.go index 866592f95d..529344a548 100644 --- a/weed/util/fullpath.go +++ b/weed/util/fullpath.go @@ -76,6 +76,15 @@ func (fp FullPath) IsLongerFileName(maxFilenameLength uint32) bool { return uint32(len([]byte(fp.Name()))) > maxFilenameLength } +func (fp FullPath) HasDotSegment() bool { + for _, seg := range strings.Split(string(fp), "/") { + if seg == "." || seg == ".." { + return true + } + } + return false +} + func (fp FullPath) Child(name string) FullPath { dir := string(fp) noPrefix := name